Short-circuit Readdir calls on overlay files when the dirent is frozen.

If we have an overlay file whose corresponding Dirent is frozen, then we should
not bother calling Readdir on the upper or lower files, since DirentReaddir
will calculate children based on the frozen Dirent tree.

A test was added that fails without this change.

PiperOrigin-RevId: 213531215
Change-Id: I4d6c98f1416541a476a34418f664ba58f936a81d

3 files changed, 109 insertions, 8 deletions

diff --git a/pkg/sentry/fs/file_overlay.go b/pkg/sentry/fs/file_overlay.go
index 113962368..41e646ee8 100644
--- a/pkg/sentry/fs/file_overlay.go
+++ b/pkg/sentry/fs/file_overlay.go
@@ -163,6 +163,21 @@ func (f *overlayFileOperations) Seek(ctx context.Context, file *File, whence See
 
 // Readdir implements FileOperations.Readdir.
 func (f *overlayFileOperations) Readdir(ctx context.Context, file *File, serializer DentrySerializer) (int64, error) {
+	root := RootFromContext(ctx)
+	defer root.DecRef()
+	dirCtx := &DirCtx{
+		Serializer: serializer,
+		DirCursor:  &f.dirCursor,
+	}
+
+	// If the directory dirent is frozen, then DirentReaddir will calculate
+	// the children based off the frozen dirent tree. There is no need to
+	// call readdir on the upper/lower layers.
+	if file.Dirent.frozen {
+		return DirentReaddir(ctx, file.Dirent, f, root, dirCtx, file.Offset())
+	}
+
+	// Otherwise proceed with usual overlay readdir.
 	o := file.Dirent.Inode.overlay
 
 	o.copyMu.RLock()
@@ -174,13 +189,6 @@ func (f *overlayFileOperations) Readdir(ctx context.Context, file *File, seriali
 		return file.Offset(), err
 	}
 
-	root := RootFromContext(ctx)
-	defer root.DecRef()
-
-	dirCtx := &DirCtx{
-		Serializer: serializer,
-		DirCursor:  &f.dirCursor,
-	}
 	return DirentReaddir(ctx, file.Dirent, f, root, dirCtx, file.Offset())
 }
 
diff --git a/pkg/sentry/fs/file_overlay_test.go b/pkg/sentry/fs/file_overlay_test.go
index 38762d8a1..830458ff9 100644
--- a/pkg/sentry/fs/file_overlay_test.go
+++ b/pkg/sentry/fs/file_overlay_test.go
@@ -174,6 +174,89 @@ func TestReaddirRevalidation(t *testing.T) {
 	}
 }
 
+// TestReaddirOverlayFrozen tests that calling Readdir on an overlay file with
+// a frozen dirent tree does not make Readdir calls to the underlying files.
+func TestReaddirOverlayFrozen(t *testing.T) {
+	ctx := contexttest.Context(t)
+
+	// Create an overlay with two directories, each with two files.
+	upper := newTestRamfsDir(ctx, []dirContent{{name: "upper-file1"}, {name: "upper-file2"}}, nil)
+	lower := newTestRamfsDir(ctx, []dirContent{{name: "lower-file1"}, {name: "lower-file2"}}, nil)
+	overlayInode := fs.NewTestOverlayDir(ctx, upper, lower, false)
+
+	// Set that overlay as the root.
+	root := fs.NewDirent(overlayInode, "root")
+	ctx = &rootContext{
+		Context: ctx,
+		root:    root,
+	}
+
+	// Check that calling Readdir on the root now returns all 4 files (2
+	// from each layer in the overlay).
+	rootFile, err := root.Inode.GetFile(ctx, root, fs.FileFlags{Read: true})
+	if err != nil {
+		t.Fatalf("root.Inode.GetFile failed: %v", err)
+	}
+	defer rootFile.DecRef()
+	ser := &fs.CollectEntriesSerializer{}
+	if err := rootFile.Readdir(ctx, ser); err != nil {
+		t.Fatalf("rootFile.Readdir failed: %v", err)
+	}
+	if got, want := ser.Order, []string{".", "..", "lower-file1", "lower-file2", "upper-file1", "upper-file2"}; !reflect.DeepEqual(got, want) {
+		t.Errorf("Readdir got names %v, want %v", got, want)
+	}
+
+	// Readdir should have been called on upper and lower.
+	upperDir := upper.InodeOperations.(*dir)
+	lowerDir := lower.InodeOperations.(*dir)
+	if !upperDir.ReaddirCalled {
+		t.Errorf("upperDir.ReaddirCalled got %v, want true", upperDir.ReaddirCalled)
+	}
+	if !lowerDir.ReaddirCalled {
+		t.Errorf("lowerDir.ReaddirCalled got %v, want true", lowerDir.ReaddirCalled)
+	}
+
+	// Reset.
+	upperDir.ReaddirCalled = false
+	lowerDir.ReaddirCalled = false
+
+	// Take references on "upper-file1" and "lower-file1", pinning them in
+	// the dirent tree.
+	for _, name := range []string{"upper-file1", "lower-file1"} {
+		if _, err := root.Walk(ctx, root, name); err != nil {
+			t.Fatalf("root.Walk(%q) failed: %v", name, err)
+		}
+		// Don't drop a reference on the returned dirent so that it
+		// will stay in the tree.
+	}
+
+	// Freeze the dirent tree.
+	root.Freeze()
+
+	// Seek back to the beginning of the file.
+	if _, err := rootFile.Seek(ctx, fs.SeekSet, 0); err != nil {
+		t.Fatalf("error seeking to beginning of directory: %v", err)
+	}
+
+	// Calling Readdir on the root now will return only the pinned
+	// children.
+	ser = &fs.CollectEntriesSerializer{}
+	if err := rootFile.Readdir(ctx, ser); err != nil {
+		t.Fatalf("rootFile.Readdir failed: %v", err)
+	}
+	if got, want := ser.Order, []string{".", "..", "lower-file1", "upper-file1"}; !reflect.DeepEqual(got, want) {
+		t.Errorf("Readdir got names %v, want %v", got, want)
+	}
+
+	// Readdir should NOT have been called on upper or lower.
+	if upperDir.ReaddirCalled {
+		t.Errorf("upperDir.ReaddirCalled got %v, want false", upperDir.ReaddirCalled)
+	}
+	if lowerDir.ReaddirCalled {
+		t.Errorf("lowerDir.ReaddirCalled got %v, want false", lowerDir.ReaddirCalled)
+	}
+}
+
 type rootContext struct {
 	context.Context
 	root *fs.Dirent
diff --git a/pkg/sentry/fs/inode_overlay_test.go b/pkg/sentry/fs/inode_overlay_test.go
index 3ee4c9667..23e5635a4 100644
--- a/pkg/sentry/fs/inode_overlay_test.go
+++ b/pkg/sentry/fs/inode_overlay_test.go
@@ -372,10 +372,14 @@ func TestCacheFlush(t *testing.T) {
 type dir struct {
 	fs.InodeOperations
 
-	// list of negative child names.
+	// List of negative child names.
 	negative []string
+
+	// Whether DeprecatedReaddir has been called on this dir.
+	ReaddirCalled bool
 }
 
+// Getxattr implements InodeOperations.Getxattr.
 func (d *dir) Getxattr(inode *fs.Inode, name string) ([]byte, error) {
 	for _, n := range d.negative {
 		if name == fs.XattrOverlayWhiteout(n) {
@@ -385,6 +389,12 @@ func (d *dir) Getxattr(inode *fs.Inode, name string) ([]byte, error) {
 	return nil, syserror.ENOATTR
 }
 
+// DeprecatedReaddir implements InodeOperations.DeprecatedReaddir.
+func (d *dir) DeprecatedReaddir(ctx context.Context, dirctx *fs.DirCtx, offset int) (int, error) {
+	d.ReaddirCalled = true
+	return d.InodeOperations.DeprecatedReaddir(ctx, dirctx, offset)
+}
+
 type dirContent struct {
 	name string
 	dir  bool