diff --git a/pkg/erofs/erofs.go b/pkg/erofs/erofs.go index f0ac37166..47cdb7b36 100644 --- a/pkg/erofs/erofs.go +++ b/pkg/erofs/erofs.go @@ -39,7 +39,7 @@ import ( ) const ( - // Definitions for super block. + // Definitions for superblock. SuperBlockMagicV1 = 0xe0f5e1e2 SuperBlockOffset = 1024 @@ -99,7 +99,7 @@ const ( DirentSize = 12 ) -// SuperBlock represents on-disk super block. +// SuperBlock represents on-disk superblock. // // +marshal // +stateify savable @@ -260,9 +260,9 @@ func (i *Image) RootNid() uint64 { return uint64(i.sb.RootNid) } -// initSuperBlock initializes the super block of this image. +// initSuperBlock initializes the superblock of this image. func (i *Image) initSuperBlock() error { - // i.sb is used in the hot path. Let's save a copy of it. + // i.sb is used in the hot path. Let's save a copy of the superblock. if err := i.unmarshalAt(&i.sb, SuperBlockOffset); err != nil { return fmt.Errorf("image size is too small") } @@ -286,7 +286,7 @@ func (i *Image) initSuperBlock() error { return nil } -// verifyChecksum verifies the checksum of the super block. +// verifyChecksum verifies the checksum of the superblock. func (i *Image) verifyChecksum() error { if i.sb.FeatureCompat&FeatureCompatSuperBlockChecksum == 0 { return nil @@ -338,7 +338,8 @@ func checkInodeAlignment(off uint64) bool { return off&((1<> bit) & ((1 << bits) - 1) } // Layout returns the inode layout. func (i *Inode) Layout() uint16 { - return bitRange(uint16(i.format), InodeLayoutBit, InodeLayoutBits) + return bitRange(i.format, InodeLayoutBit, InodeLayoutBits) } // DataLayout returns the inode data layout. func (i *Inode) DataLayout() uint16 { - return bitRange(uint16(i.format), InodeDataLayoutBit, InodeDataLayoutBits) + return bitRange(i.format, InodeDataLayoutBit, InodeDataLayoutBits) } // IsRegular indicates whether i represents a regular file. @@ -706,6 +708,13 @@ func (i *Inode) IterDirents(cb func(name string, typ uint8, nid uint64) error) e // Iterate all the blocks which contain dirents. for blocks > 0 { + // Get the max data size of this block. + maxSize := blockSize + if blocks == 1 { + if tailSize := uint32(i.size) & (blockSize - 1); tailSize != 0 { + maxSize = tailSize + } + } // Get the first dirent in the current block. direntOff := start d, err := i.image.direntAt(direntOff) @@ -714,13 +723,11 @@ func (i *Inode) IterDirents(cb func(name string, typ uint8, nid uint64) error) e } // Apart from the offset of the first filename, nameOff0 also indicates // the total number of dirents in this block. - nameOff0 := start + uint64(d.NameOff) - maxSize := blockSize - if blocks == 1 { - if tailSize := uint32(i.size) & (blockSize - 1); tailSize != 0 { - maxSize = tailSize - } + if d.NameOff < DirentSize || uint32(d.NameOff) >= maxSize { + log.Warningf("Invalid nameOff0 %v at inode (nid=%v)", d.NameOff, i.Nid()) + return linuxerr.EUCLEAN } + nameOff0 := start + uint64(d.NameOff) // Iterate all the dirents in this block. for d != nil { var ( @@ -740,20 +747,22 @@ func (i *Inode) IterDirents(cb func(name string, typ uint8, nid uint64) error) e } nameLen = uint32(next.NameOff - d.NameOff) } - + if uint32(d.NameOff)+nameLen > maxSize || nameLen > MaxNameLen || nameLen == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return linuxerr.EUCLEAN + } buf, err := i.image.BytesAt(start+uint64(d.NameOff), uint64(nameLen)) if err != nil { return err } if lastDirent { - if n := bytes.IndexByte(buf, 0); n != -1 { + if n := bytes.IndexByte(buf, 0); n == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return linuxerr.EUCLEAN + } else if n != -1 { nameLen = uint32(n) } } - if nameLen > MaxNameLen { - log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) - return linuxerr.EUCLEAN - } name := string(buf[:nameLen]) if err := cb(name, d.FileType, d.Nid()); err != nil { return err diff --git a/pkg/erofs/erofs_test.go b/pkg/erofs/erofs_test.go index d29b3b87e..a07e87f7e 100644 --- a/pkg/erofs/erofs_test.go +++ b/pkg/erofs/erofs_test.go @@ -20,7 +20,7 @@ import ( func TestOnDiskStructureSizes(t *testing.T) { if sb := new(SuperBlock); sb.SizeBytes() != SuperBlockSize { - t.Errorf("wrong super block size: want %d, got %d", SuperBlockSize, sb.SizeBytes()) + t.Errorf("wrong superblock size: want %d, got %d", SuperBlockSize, sb.SizeBytes()) } if i := new(InodeCompact); i.SizeBytes() != InodeCompactSize { diff --git a/pkg/erofs/erofs_unsafe.go b/pkg/erofs/erofs_unsafe.go index c1c37dbc3..4dcf912ca 100644 --- a/pkg/erofs/erofs_unsafe.go +++ b/pkg/erofs/erofs_unsafe.go @@ -16,6 +16,13 @@ package erofs import "unsafe" +// pointerAt returns a pointer to offset off within the memory backed by image. +// +// Precondition: Callers are responsible for the range check. func (i *Image) pointerAt(off uint64) unsafe.Pointer { + // Although callers will always do the range check, there is no need to + // bother with the slice's builtin range check below. Because this function + // will be inlined into callers, and there are no redundant checks and + // unnecessary out-of-range panic calls in the code generated by the compiler. return unsafe.Pointer(&i.bytes[off]) } diff --git a/pkg/sentry/fsimpl/erofs/erofs.go b/pkg/sentry/fsimpl/erofs/erofs.go index 24e003cea..658c3ba4b 100644 --- a/pkg/sentry/fsimpl/erofs/erofs.go +++ b/pkg/sentry/fsimpl/erofs/erofs.go @@ -32,6 +32,8 @@ import ( "gvisor.dev/gvisor/pkg/sentry/vfs" ) +// Name is the filesystem name. It is part of the interface used by users, +// e.g. via annotations, and shouldn't change. const Name = "erofs" // Mount option names for EROFS. diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 66dbd81fb..a63180065 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -3049,24 +3049,44 @@ func TestExecFDExec(t *testing.T) { } } -// TestMountEROFS checks that the checksums from the target directory in container -// are identical with the ones from the source directory on host. -func TestMountEROFS(t *testing.T) { - // Skip this test if mkfs.erofs is not available. +// skipIfNotAvailable skips the test if the requested executable files are not available. +func skipIfNotAvailable(t *testing.T, files ...string) { + for _, f := range files { + if _, err := exec.LookPath(f); err != nil { + t.Skipf("%v is not available: %v", f, err) + } + } +} + +// createImageEROFS creates the EROFS image from the source directory using the requested options. +func createImageEROFS(image, source string, options ...string) error { mkfs, err := exec.LookPath("mkfs.erofs") if err != nil { - t.Skipf("mkfs.erofs is not available: %v", err) + return fmt.Errorf("mkfs.erofs is not available: %v", err) } + cmd := fmt.Sprintf("%s %s %s %s", mkfs, strings.Join(options, " "), image, source) + if out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput(); err != nil { + return fmt.Errorf("exec: sh -c %q, err: %v, out: %s", cmd, err, out) + } + return nil +} + +// TestMountEROFS checks that the checksums from the target directory in the container +// are identical with the ones from the source directory on the host. +func TestMountEROFS(t *testing.T) { + // Skip this test if mkfs.erofs is not available. + skipIfNotAvailable(t, "mkfs.erofs") // Create a temporary directory to save the test files. - assetsDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs-assets") + testDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs_mount_test_") if err != nil { t.Fatalf("ioutil.TempDir() failed: %v", err) } + defer os.RemoveAll(testDir) // Create a temporary directory with some random files in it, which will // be used as the source directory to create the EROFS images. - sourceDir := filepath.Join(assetsDir, "source") + sourceDir := filepath.Join(testDir, "source") if err := os.Mkdir(sourceDir, 0755); err != nil { t.Fatalf("os.Mkdir() failed: %v", err) } @@ -3092,7 +3112,7 @@ func TestMountEROFS(t *testing.T) { // Create a test script which can be used to get the checksums // from a specified directory. - scriptFile := filepath.Join(assetsDir, "test-script") + scriptFile := filepath.Join(testDir, "test-script") if err := os.WriteFile(scriptFile, []byte(`#!/bin/bash set -u -e -o pipefail dir=$1 @@ -3102,7 +3122,7 @@ find $dir -type l -o -type f | sort | xargs cat | md5sum`), 0755); err != nil { t.Fatalf("os.WriteFile() failed: %v", err) } - // Get the checksums from the source directory on host. + // Get the checksums from the source directory on the host. var checksums string if out, err := exec.Command(scriptFile, sourceDir).CombinedOutput(); err != nil { t.Fatalf("exec: %s %s, err: %v, out: %s", scriptFile, sourceDir, err, out) @@ -3138,9 +3158,8 @@ find $dir -type l -o -type f | sort | xargs cat | md5sum`), 0755); err != nil { // Create the EROFS images. for _, i := range images { - cmd := fmt.Sprintf("%s %s %s %s", mkfs, i.options, filepath.Join(assetsDir, i.name), sourceDir) - if out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput(); err != nil { - t.Fatalf("exec: sh -c %q, err: %v, out: %s", cmd, err, out) + if err := createImageEROFS(filepath.Join(testDir, i.name), sourceDir, i.options); err != nil { + t.Fatalf("error creating EROFS image: %v", err) } } @@ -3169,21 +3188,21 @@ find $dir -type l -o -type f | sort | xargs cat | md5sum`), 0755); err != nil { targetDir := "/mnt" for _, i := range images { - // Mount the EROFS image in container. - imageFile := filepath.Join(assetsDir, i.name) - if err := c.Sandbox.Mount(c.ID, "erofs", imageFile, targetDir); err != nil { - t.Fatalf("error mounting EROFS image %q to %q, err: %v", imageFile, targetDir, err) + // Mount the EROFS image in the container. + imageFile := filepath.Join(testDir, i.name) + if err := c.Sandbox.Mount(c.ID, erofs.Name, imageFile, targetDir); err != nil { + t.Fatalf("error mounting EROFS image %q at %q, err: %v", imageFile, targetDir, err) } - // Get the checksums from the target directory in container, and check if they are - // identical with the ones got from the source directory on host. + // Get the checksums from the target directory in the container, and check if they are + // identical with the ones got from the source directory on the host. if out, err := executeCombinedOutput(conf, c, nil, scriptFile, targetDir); err != nil { t.Fatalf("exec: %s %s, err: %v, out: %s", scriptFile, targetDir, err, out) } else if checksums != string(out) { t.Errorf("checksums do not match, got: %s from %s, expected: %s", out, imageFile, checksums) } - // Unmount the EROFS image in container. + // Unmount the EROFS image in the container. if out, err := executeCombinedOutput(conf, c, nil, "/bin/umount", targetDir); err != nil { t.Fatalf("exec: umount %q, err: %v, out: %s", targetDir, err, out) } @@ -3193,21 +3212,15 @@ find $dir -type l -o -type f | sort | xargs cat | md5sum`), 0755); err != nil { // createRootfsEROFS creates a rootfs directory and an EROFS rootfs image in // the directory dir. func createRootfsEROFS(dir string) (string, string, error) { - mkfs, err := exec.LookPath("mkfs.erofs") - if err != nil { - return "", "", fmt.Errorf("mkfs.erofs is not available: %v", err) - } - - busybox, err := exec.LookPath("busybox") - if err != nil { - return "", "", fmt.Errorf("busybox is not available: %v", err) - } - // Create a rootfs directory with busybox in root. rootfsDir := filepath.Join(dir, "rootfs") if err := os.Mkdir(rootfsDir, 0755); err != nil { return "", "", fmt.Errorf("os.Mkdir() failed: %v", err) } + busybox, err := exec.LookPath("busybox") + if err != nil { + return "", "", fmt.Errorf("busybox is not available: %v", err) + } if err := testutil.Copy(busybox, filepath.Join(rootfsDir, "busybox")); err != nil { return "", "", fmt.Errorf("failed to copy busybox: %v", err) } @@ -3223,29 +3236,24 @@ func createRootfsEROFS(dir string) (string, string, error) { // Build the EROFS rootfs image. rootfsImage := filepath.Join(dir, "rootfs.img") - cmd := fmt.Sprintf("%s -E noinline_data %s %s", mkfs, rootfsImage, rootfsDir) - if out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput(); err != nil { - return "", "", fmt.Errorf("exec: sh -c %q, err: %v, out: %s", cmd, err, out) + if err := createImageEROFS(rootfsImage, rootfsDir, "-E noinline_data"); err != nil { + return "", "", fmt.Errorf("error creating EROFS image: %v", err) } return rootfsDir, rootfsImage, nil } // TestRootfsEROFS starts a container using an EROFS image as the rootfs and checks that -// the rootfs in container is an EROFS. +// the rootfs in the container is an EROFS. func TestRootfsEROFS(t *testing.T) { // Skip this test if mkfs.erofs or busybox are not available. - if _, err := exec.LookPath("mkfs.erofs"); err != nil { - t.Skipf("mkfs.erofs is not available: %v", err) - } - if _, err := exec.LookPath("busybox"); err != nil { - t.Skipf("busybox is not available: %v", err) - } + skipIfNotAvailable(t, "mkfs.erofs", "busybox") testDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs_rootfs_test_") if err != nil { t.Fatalf("ioutil.TempDir() failed: %v", err) } + defer os.RemoveAll(testDir) rootfsDir, rootfsImage, err := createRootfsEROFS(testDir) if err != nil { @@ -3308,17 +3316,13 @@ func TestRootfsEROFS(t *testing.T) { // an EROFS image as the rootfs. func TestCheckpointRestoreEROFS(t *testing.T) { // Skip this test if mkfs.erofs or busybox are not available. - if _, err := exec.LookPath("mkfs.erofs"); err != nil { - t.Skipf("mkfs.erofs is not available: %v", err) - } - if _, err := exec.LookPath("busybox"); err != nil { - t.Skipf("busybox is not available: %v", err) - } + skipIfNotAvailable(t, "mkfs.erofs", "busybox") testDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs_checkpoint_restore_test_") if err != nil { t.Fatalf("ioutil.TempDir() failed: %v", err) } + defer os.RemoveAll(testDir) rootfsDir, rootfsImage, err := createRootfsEROFS(testDir) if err != nil {