From 3bcfd779296949aaca8706abaa3f4a2e801c343f Mon Sep 17 00:00:00 2001 From: Tiwei Bie Date: Sat, 11 Nov 2023 08:28:52 +0800 Subject: [PATCH 1/2] erofs: cleanups and hardening - Add more sanity checks on dirent name offset and length. - Both of "super block" and "superblock" are used in comments and strings now, let's convert all "super block" to "superblock". - Add some comments that can add clarity. - Refactor the code in tests to make it easier to add more tests. Signed-off-by: Tiwei Bie --- pkg/erofs/erofs.go | 61 +++++++++++--------- pkg/erofs/erofs_test.go | 2 +- pkg/erofs/erofs_unsafe.go | 7 +++ pkg/sentry/fsimpl/erofs/erofs.go | 2 + runsc/container/container_test.go | 94 ++++++++++++++++--------------- 5 files changed, 94 insertions(+), 72 deletions(-) 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 { From 0da79ed4bf4aa746ab3036505c8aa9c9f3c2bd14 Mon Sep 17 00:00:00 2001 From: Tiwei Bie Date: Sat, 11 Nov 2023 09:28:19 +0800 Subject: [PATCH 2/2] erofs: support block based dirent lookup This patch adds the block based dirent lookup support to pkg/erofs, which will do the dirent lookup by doing binary search on disk data directly. This is helpful for searching files in large directories and also reduces the memory overhead. Signed-off-by: Tiwei Bie --- pkg/erofs/BUILD | 1 + pkg/erofs/erofs.go | 271 ++++++++++++++++++--------- pkg/erofs/erofs_unsafe.go | 9 + pkg/sentry/fsimpl/erofs/directory.go | 18 +- runsc/container/container_test.go | 118 ++++++++++++ 5 files changed, 326 insertions(+), 91 deletions(-) diff --git a/pkg/erofs/BUILD b/pkg/erofs/BUILD index b517ee3ed..ce8779eb6 100644 --- a/pkg/erofs/BUILD +++ b/pkg/erofs/BUILD @@ -17,6 +17,7 @@ go_library( "//pkg/abi/linux", "//pkg/cleanup", "//pkg/errors/linuxerr", + "//pkg/gohacks", "//pkg/hostarch", "//pkg/log", "//pkg/marshal", diff --git a/pkg/erofs/erofs.go b/pkg/erofs/erofs.go index 47cdb7b36..08e9d6d75 100644 --- a/pkg/erofs/erofs.go +++ b/pkg/erofs/erofs.go @@ -32,6 +32,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/gohacks" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/marshal" @@ -467,13 +468,15 @@ func (i *Image) Inode(nid uint64) (Inode, error) { return Inode{}, linuxerr.ENOTSUP } + blockSize := uint64(i.BlockSize()) + inode.blocks = (inode.size + (blockSize - 1)) / blockSize + switch dataLayout := inode.DataLayout(); dataLayout { case InodeDataLayoutFlatInline: // Check that whether the file data in the last block fits into // the remaining room of the metadata block. - blockSize := i.BlockSize() - tailSize := uint32(inode.size) & (blockSize - 1) - if tailSize == 0 || tailSize > blockSize-uint32(inodeSize) { + tailSize := inode.size & (blockSize - 1) + if tailSize == 0 || tailSize > blockSize-uint64(inodeSize) { log.Warningf("Inline data not found or cross block boundary at inode (nid=%v)", nid) return Inode{}, linuxerr.EUCLEAN } @@ -506,6 +509,11 @@ type Inode struct { // if it's not zero in the metadata block. idataOff uint64 + // blocks indicates the count of blocks that store the data associated + // with this inode. It will count in the metadata block that includes + // the inline data as well. + blocks uint64 + // format is the format of this inode. format uint16 @@ -656,17 +664,41 @@ func (i *Inode) Data() (safemem.BlockSeq, error) { } } -// blocks returns the number of blocks that contain data. It will count in the -// metadata block which contains the inline data. -func (i *Inode) blocks() uint64 { - blockSize := uint64(i.image.BlockSize()) - return (i.size + (blockSize - 1)) / blockSize +// blockData represents the information of the data in a block. +type blockData struct { + // base indicates the data offset within the image. + base uint64 + // size indicates the data size. + size uint32 } -// IterDirents invokes cb on each entry in the directory represented by this inode. -// The directory entries will be iterated in alphabetical order. +// valid indicates whether this is valid information about the data in a block. +func (b *blockData) valid() bool { + // The data offset within the image will never be zero. + return b.base > 0 +} + +// getBlockDataInfo returns the information of the data in the block identified by +// blockIdx of this inode. // -// https://docs.kernel.org/filesystems/erofs.html#directories +// Precondition: blockIdx < i.blocks. +func (i *Inode) getBlockDataInfo(blockIdx uint64) blockData { + blockSize := i.image.BlockSize() + lastBlock := blockIdx == i.blocks-1 + base := i.idataOff + if !lastBlock || base == 0 { + base = i.dataOff + blockIdx*uint64(blockSize) + } + size := blockSize + if lastBlock { + if tailSize := uint32(i.size) & (blockSize - 1); tailSize != 0 { + size = tailSize + } + } + return blockData{base, size} +} + +// getDirentName returns the name of dirent d in the given block of this inode. // // The on-disk format of one block looks like this: // @@ -692,94 +724,163 @@ func (i *Inode) blocks() uint64 { // // [ (metadata block) inode | optional fields | dirent M+2 | dirent M+3 | name M+2 | name M+3 | optional padding ] // -// All directory entries are _strictly_ recorded in alphabetical order. +// Refer: https://docs.kernel.org/filesystems/erofs.html#directories +func (i *Inode) getDirentName(d *Dirent, block blockData, lastDirent bool) ([]byte, error) { + var nameLen uint32 + if lastDirent { + nameLen = block.size - uint32(d.NameOff) + } else { + nameLen = uint32(direntAfter(d).NameOff - d.NameOff) + } + if uint32(d.NameOff)+nameLen > block.size || nameLen > MaxNameLen || nameLen == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return nil, linuxerr.EUCLEAN + } + name, err := i.image.BytesAt(block.base+uint64(d.NameOff), uint64(nameLen)) + if err != nil { + return nil, err + } + if lastDirent { + // Optional padding may exist at the end of a block. + n := bytes.IndexByte(name, 0) + if n == 0 { + log.Warningf("Corrupted dirent at inode (nid=%v)", i.Nid()) + return nil, linuxerr.EUCLEAN + } + if n != -1 { + name = name[:n] + } + } + return name, nil +} + +// getDirent0 returns a pointer to the first dirent in the given block of this inode. +func (i *Inode) getDirent0(block blockData) (*Dirent, error) { + d0, err := i.image.direntAt(block.base) + if err != nil { + return nil, err + } + if d0.NameOff < DirentSize || uint32(d0.NameOff) >= block.size { + log.Warningf("Invalid nameOff0 %v at inode (nid=%v)", d0.NameOff, i.Nid()) + return nil, linuxerr.EUCLEAN + } + return d0, nil +} + +// Lookup looks up a child by the name. The child inode number will be returned on success. +func (i *Inode) Lookup(name string) (uint64, error) { + if !i.IsDir() { + return 0, linuxerr.ENOTDIR + } + + // Currently (Go 1.21), there is no safe and efficient way to do three-way + // string comparisons, so let's convert the string to a byte slice first. + nameBytes := gohacks.ImmutableBytesFromString(name) + + // In EROFS, all directory entries are _strictly_ recorded in alphabetical + // order. The lookup is done by directly performing binary search on the + // disk data similar to what Linux does in fs/erofs/namei.c:erofs_namei(). + var ( + targetBlock blockData + targetNumDirents uint16 + ) + + // Find the block that may contain the target dirent first. + bLeft, bRight := int64(0), int64(i.blocks)-1 + for bLeft <= bRight { + // Cast to uint64 to avoid overflow. + mid := uint64(bLeft+bRight) >> 1 + block := i.getBlockDataInfo(mid) + d0, err := i.getDirent0(block) + if err != nil { + return 0, err + } + numDirents := d0.NameOff / DirentSize + d0Name, err := i.getDirentName(d0, block, numDirents == 1) + if err != nil { + return 0, err + } + switch bytes.Compare(nameBytes, d0Name) { + case 0: + // Found the target dirent. + return d0.Nid(), nil + case 1: + // name > d0Name, this block may contain the target dirent. + targetBlock = block + targetNumDirents = numDirents + bLeft = int64(mid) + 1 + case -1: + // name < d0Name, this is not the block we're looking for. + bRight = int64(mid) - 1 + } + } + + if !targetBlock.valid() { + // The target block was not found. + return 0, linuxerr.ENOENT + } + + // Find the target dirent in the target block. Note that, as the 0th dirent + // has already been checked during the block binary search, we don't need to + // check it again and can define dLeft/dRight as unsigned types. + dLeft, dRight := uint16(1), targetNumDirents-1 + for dLeft <= dRight { + // The sum will never lead to a uint16 overflow, as the maximum value of + // the operands is MaxUint16/DirentSize. + mid := (dLeft + dRight) >> 1 + direntOff := targetBlock.base + uint64(mid)*DirentSize + d, err := i.image.direntAt(direntOff) + if err != nil { + return 0, err + } + dName, err := i.getDirentName(d, targetBlock, mid == targetNumDirents-1) + if err != nil { + return 0, err + } + switch bytes.Compare(nameBytes, dName) { + case 0: + // Found the target dirent. + return d.Nid(), nil + case 1: + // name > dName. + dLeft = mid + 1 + case -1: + // name < dName. + dRight = mid - 1 + } + } + + return 0, linuxerr.ENOENT +} + +// IterDirents invokes cb on each entry in the directory represented by this inode. +// The directory entries will be iterated in alphabetical order. func (i *Inode) IterDirents(cb func(name string, typ uint8, nid uint64) error) error { if !i.IsDir() { return linuxerr.ENOTDIR } - blocks := i.blocks() - blockSize := i.image.BlockSize() - - start := i.dataOff - if blocks == 1 && i.idataOff != 0 { - start = i.idataOff - } - // 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) + for blockIdx := uint64(0); blockIdx < i.blocks; blockIdx++ { + block := i.getBlockDataInfo(blockIdx) + d, err := i.getDirent0(block) if err != nil { return err } - // Apart from the offset of the first filename, nameOff0 also indicates - // the total number of dirents in this block. - 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 ( - next *Dirent - nameLen uint32 - ) - direntOff += uint64(d.SizeBytes()) - lastDirent := direntOff >= nameOff0 - if lastDirent { - // There is no more dirent in this block, d is the last one. - next = nil - nameLen = maxSize - uint32(d.NameOff) - } else { - // Get the next adjacent dirent. - if next, err = i.image.direntAt(direntOff); err != nil { - return err - } - 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)) + numDirents := d.NameOff / DirentSize + for { + name, err := i.getDirentName(d, block, numDirents == 1) if err != nil { return err } - if lastDirent { - 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) - } - } - name := string(buf[:nameLen]) - if err := cb(name, d.FileType, d.Nid()); err != nil { + if err := cb(string(name), d.FileType, d.Nid()); err != nil { return err } - - // Go on to process the next adjacent dirent. - d = next - } - - blocks-- - - if blocks == 1 && i.idataOff != 0 { - // If we have any inline data and this is the last block, we need to process it now. - start = i.idataOff - } else { - // Just go on to process the next adjacent block. - start += uint64(blockSize) + if numDirents--; numDirents == 0 { + break + } + d = direntAfter(d) } } return nil @@ -794,7 +895,7 @@ func (i *Inode) Readlink() (string, error) { size := i.size if i.idataOff != 0 { // Inline symlink data shouldn't cross block boundary. - if i.blocks() > 1 { + if i.blocks > 1 { log.Warningf("Inline data cross block boundary at inode (nid=%v)", i.Nid()) return "", linuxerr.EUCLEAN } diff --git a/pkg/erofs/erofs_unsafe.go b/pkg/erofs/erofs_unsafe.go index 4dcf912ca..fffc44424 100644 --- a/pkg/erofs/erofs_unsafe.go +++ b/pkg/erofs/erofs_unsafe.go @@ -26,3 +26,12 @@ func (i *Image) pointerAt(off uint64) unsafe.Pointer { // unnecessary out-of-range panic calls in the code generated by the compiler. return unsafe.Pointer(&i.bytes[off]) } + +// direntAfter returns a pointer to the next adjacent dirent after dirent d. +// +// Preconditions: +// - d is a pointer to the memory backed by image. +// - d is not the last dirent in its block. +func direntAfter(d *Dirent) *Dirent { + return (*Dirent)(unsafe.Pointer(uintptr(unsafe.Pointer(d)) + DirentSize)) +} diff --git a/pkg/sentry/fsimpl/erofs/directory.go b/pkg/sentry/fsimpl/erofs/directory.go index fadc584f3..13203375a 100644 --- a/pkg/sentry/fsimpl/erofs/directory.go +++ b/pkg/sentry/fsimpl/erofs/directory.go @@ -61,12 +61,18 @@ func (i *inode) getDirents() ([]vfs.Dirent, error) { } func (i *inode) lookup(name string) (uint64, error) { - // TODO: For simplicity, currently a lookup will cause all dirents to be - // read and cached. But it hurts the performance of large directories. - // We should do binary search on disk data directly (like Linux does). - dirents, err := i.getDirents() - if err != nil { - return 0, err + var dirents []vfs.Dirent + + // Lazily fetch dirents. + if i.dirMu.TryRLock() { + dirents = i.dirents // +checklocksforce: TryRLock. + i.dirMu.RUnlock() // +checklocksforce: TryRLock. + } + + if dirents == nil { + // The dirents cache is not available immediately, let's do + // binary search on disk data directly. + return i.Lookup(name) } // The dirents are sorted in alphabetical order. We do binary search diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index a63180065..5b08a132f 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -3353,3 +3353,121 @@ func TestCheckpointRestoreEROFS(t *testing.T) { }) } } + +// TestLookupEROFS reads the files in EROFS images, which contain some random files, +// and checks if the data is as expected. +func TestLookupEROFS(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. + testDir, err := ioutil.TempDir(testutil.TmpDir(), "erofs_lookup_test_") + if err != nil { + t.Fatalf("ioutil.TempDir() failed: %v", err) + } + defer os.RemoveAll(testDir) + + spec, _ := sleepSpecConf(t) + conf := testutil.TestConfig(t) + _, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatalf("error setting up container: %v", err) + } + defer cleanup() + + // Create and start the container. + args := Args{ + ID: testutil.RandomContainerID(), + Spec: spec, + BundleDir: bundleDir, + } + c, err := New(conf, args) + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer c.Destroy() + if err := c.Start(conf); err != nil { + t.Fatalf("error starting container: %v", err) + } + + tcs := []struct { + name string + size int + }{ + { + name: "tiny", + size: 1, + }, + { + name: "small", + size: 10, + }, + { + name: "medium", + size: 100, + }, + { + name: "large", + size: 1000, + }, + } + + targetDir := "/mnt" + for _, tc := range tcs { + // Add some randomness to the number of files. + size := tc.size + rand.Intn(tc.size) + + // Create a temporary directory with some random files in it, which will + // be used as the source directory to create the EROFS image. + sourceDir := filepath.Join(testDir, tc.name) + if err := os.Mkdir(sourceDir, 0755); err != nil { + t.Fatalf("os.Mkdir() failed: %v", err) + } + randomFiles := make([]string, 0, size) + for i := 0; i < size; i++ { + file, err := ioutil.TempFile(sourceDir, "") + if err != nil { + t.Fatalf("ioutil.TempFile() failed: %v", err) + } + name := filepath.Base(file.Name()) + if _, err := file.Write([]byte(name)); err != nil { + t.Fatalf("file.Write() failed: %v", err) + } + file.Close() + randomFiles = append(randomFiles, name) + } + + // Create the EROFS image. + imageFile := filepath.Join(testDir, fmt.Sprintf("%s.img", tc.name)) + if err := createImageEROFS(imageFile, sourceDir); err != nil { + t.Fatalf("error creating EROFS image: %v", err) + } + + // Mount the EROFS image in the container. + 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) + } + + // Read the files in the EROFS image and check if the data is as expected. + for i, inc := 0, max(size/100, 1); i < size; i += inc { + targetFile := randomFiles[i] + cmd := fmt.Sprintf("cat %s", filepath.Join(targetDir, targetFile)) + if out, err := executeCombinedOutput(conf, c, nil, "/bin/sh", "-c", cmd); err != nil { + t.Fatalf("exec: sh -c %q, err: %v, out: %s", cmd, err, out) + } else if targetFile != string(out) { + t.Errorf("file does not match, got: %s, expected: %s", out, targetFile) + } + } + + // Test for the read failure with a non-existent file. + cmd := fmt.Sprintf("cat %s", filepath.Join(targetDir, "nonexist")) + if out, err := executeCombinedOutput(conf, c, nil, "/bin/sh", "-c", cmd); err == nil { + t.Errorf("exec: sh -c %q, succeeded to read the non-existent file: %s", cmd, out) + } + + // 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) + } + } +}