From 7c3ff55fab59c13929442089fdcf96cbc98d2bb5 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Tue, 15 Nov 2022 13:13:20 -0800 Subject: [PATCH] Update fd_table_test to use VFS2. This unit test had been using VFS1. Updates #1624 PiperOrigin-RevId: 488740255 --- pkg/sentry/kernel/BUILD | 3 +- pkg/sentry/kernel/fd_table.go | 6 +- pkg/sentry/kernel/fd_table_test.go | 111 +++++++++++------- .../vfs/file_description_impl_util_test.go | 13 +- 4 files changed, 72 insertions(+), 61 deletions(-) diff --git a/pkg/sentry/kernel/BUILD b/pkg/sentry/kernel/BUILD index 2100c2009..cd74c613b 100644 --- a/pkg/sentry/kernel/BUILD +++ b/pkg/sentry/kernel/BUILD @@ -387,13 +387,12 @@ go_test( "//pkg/hostarch", "//pkg/sentry/arch", "//pkg/sentry/contexttest", - "//pkg/sentry/fs", - "//pkg/sentry/fs/filetest", "//pkg/sentry/kernel/sched", "//pkg/sentry/limits", "//pkg/sentry/pgalloc", "//pkg/sentry/time", "//pkg/sentry/usage", + "//pkg/sentry/vfs", "//pkg/sync", ], ) diff --git a/pkg/sentry/kernel/fd_table.go b/pkg/sentry/kernel/fd_table.go index 258d23580..611ed83c0 100644 --- a/pkg/sentry/kernel/fd_table.go +++ b/pkg/sentry/kernel/fd_table.go @@ -381,7 +381,7 @@ func (f *FDTable) NewFDsVFS2(ctx context.Context, minFD int32, files []*vfs.File if lim.Cur != limits.Infinity { end = int32(lim.Cur) } - if minFD >= end { + if minFD+int32(len(files)) > end { return nil, unix.EMFILE } } @@ -539,8 +539,8 @@ func (f *FDTable) SetFlagsForRange(ctx context.Context, startFd int32, endFd int for fd, err := f.fdBitmap.FirstOne(uint32(startFd)); err == nil && fd <= uint32(endFd); fd, err = f.fdBitmap.FirstOne(fd + 1) { fdI32 := int32(fd) - file, _, _ := f.get(fdI32) - f.set(ctx, fdI32, file, flags) + fd, _, _ := f.getVFS2(fdI32) + f.setVFS2(ctx, fdI32, fd, flags) } return nil diff --git a/pkg/sentry/kernel/fd_table_test.go b/pkg/sentry/kernel/fd_table_test.go index 374ff3edb..0fb5e5723 100644 --- a/pkg/sentry/kernel/fd_table_test.go +++ b/pkg/sentry/kernel/fd_table_test.go @@ -20,9 +20,8 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/contexttest" - "gvisor.dev/gvisor/pkg/sentry/fs" - "gvisor.dev/gvisor/pkg/sentry/fs/filetest" "gvisor.dev/gvisor/pkg/sentry/limits" + "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" ) @@ -33,7 +32,26 @@ const ( maxFD = 2 * 1024 ) -func runTest(t testing.TB, fn func(ctx context.Context, fdTable *FDTable, file *fs.File, limitSet *limits.LimitSet)) { +// testFD is a read-only FileDescriptionImpl representing a regular file. +type testFD struct { + vfsfd vfs.FileDescription + vfs.FileDescriptionDefaultImpl + vfs.DentryMetadataFileDescriptionImpl + vfs.NoLockFD +} + +// Release implements FileDescriptionImpl.Release. +func (fd *testFD) Release(context.Context) {} + +func newTestFD(ctx context.Context, vfsObj *vfs.VirtualFilesystem) *vfs.FileDescription { + vd := vfsObj.NewAnonVirtualDentry("testFD") + defer vd.DecRef(ctx) + var fd testFD + fd.vfsfd.Init(&fd, 0 /* flags */, vd.Mount(), vd.Dentry(), &vfs.FileDescriptionOptions{}) + return &fd.vfsfd +} + +func runTest(t testing.TB, fn func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, limitSet *limits.LimitSet)) { t.Helper() // Don't show in stacks. // Create the limits and context. @@ -41,55 +59,60 @@ func runTest(t testing.TB, fn func(ctx context.Context, fdTable *FDTable, file * limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD}, true) ctx := contexttest.WithLimitSet(contexttest.Context(t), limitSet) - // Create a test file.; - file := filetest.NewTestFile(t) + vfsObj := &vfs.VirtualFilesystem{} + if err := vfsObj.Init(ctx); err != nil { + t.Fatalf("VFS init: %v", err) + } + + fd := newTestFD(ctx, vfsObj) + defer fd.DecRef(ctx) // Create the table. fdTable := new(FDTable) fdTable.init() // Run the test. - fn(ctx, fdTable, file, limitSet) + fn(ctx, fdTable, fd, limitSet) } // TestFDTableMany allocates maxFD FDs, i.e. maxes out the FDTable, until there // is no room, then makes sure that NewFDAt works and also that if we remove // one and add one that works too. func TestFDTableMany(t *testing.T) { - runTest(t, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { + runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { for i := 0; i < maxFD; i++ { - if _, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil { + if _, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil { t.Fatalf("Allocated %v FDs but wanted to allocate %v", i, maxFD) } } - if _, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err == nil { + if _, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err == nil { t.Fatalf("fdTable.NewFDs(0, r) in full map: got nil, wanted error") } - if err := fdTable.NewFDAt(ctx, 1, file, FDFlags{}); err != nil { + if err := fdTable.NewFDAtVFS2(ctx, 1, fd, FDFlags{}); err != nil { t.Fatalf("fdTable.NewFDAt(1, r, FDFlags{}): got %v, wanted nil", err) } i := int32(2) fdTable.Remove(ctx, i) - if fds, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil || fds[0] != i { + if fds, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil || fds[0] != i { t.Fatalf("Allocated %v FDs but wanted to allocate %v: %v", i, maxFD, err) } }) } func TestFDTableOverLimit(t *testing.T) { - runTest(t, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { - if _, err := fdTable.NewFDs(ctx, maxFD, []*fs.File{file}, FDFlags{}); err == nil { + runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { + if _, err := fdTable.NewFDsVFS2(ctx, maxFD, []*vfs.FileDescription{fd}, FDFlags{}); err == nil { t.Fatalf("fdTable.NewFDs(maxFD, f): got nil, wanted error") } - if _, err := fdTable.NewFDs(ctx, maxFD-2, []*fs.File{file, file, file}, FDFlags{}); err == nil { + if _, err := fdTable.NewFDsVFS2(ctx, maxFD-2, []*vfs.FileDescription{fd, fd, fd}, FDFlags{}); err == nil { t.Fatalf("fdTable.NewFDs(maxFD-2, {f,f,f}): got nil, wanted error") } - if fds, err := fdTable.NewFDs(ctx, maxFD-3, []*fs.File{file, file, file}, FDFlags{}); err != nil { + if fds, err := fdTable.NewFDsVFS2(ctx, maxFD-3, []*vfs.FileDescription{fd, fd, fd}, FDFlags{}); err != nil { t.Fatalf("fdTable.NewFDs(maxFD-3, {f,f,f}): got %v, wanted nil", err) } else { for _, fd := range fds { @@ -97,11 +120,11 @@ func TestFDTableOverLimit(t *testing.T) { } } - if fds, err := fdTable.NewFDs(ctx, maxFD-1, []*fs.File{file}, FDFlags{}); err != nil || fds[0] != maxFD-1 { + if fds, err := fdTable.NewFDsVFS2(ctx, maxFD-1, []*vfs.FileDescription{fd}, FDFlags{}); err != nil || fds[0] != maxFD-1 { t.Fatalf("fdTable.NewFDAt(1, r, FDFlags{}): got %v, wanted nil", err) } - if fds, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil { + if fds, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil { t.Fatalf("Adding an FD to a resized map: got %v, want nil", err) } else if len(fds) != 1 || fds[0] != 0 { t.Fatalf("Added an FD to a resized map: got %v, want {1}", fds) @@ -113,64 +136,64 @@ func TestFDTableOverLimit(t *testing.T) { // GetRefs, and DecRefs work. The ordering is just weird enough that a // table-driven approach seemed clumsy. func TestFDTable(t *testing.T) { - runTest(t, func(ctx context.Context, fdTable *FDTable, file *fs.File, limitSet *limits.LimitSet) { + runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, limitSet *limits.LimitSet) { // Cap the limit at one. limitSet.Set(limits.NumberOfFiles, limits.Limit{1, maxFD}, true) - if _, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil { + if _, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil { t.Fatalf("Adding an FD to an empty 1-size map: got %v, want nil", err) } - if _, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err == nil { + if _, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err == nil { t.Fatalf("Adding an FD to a filled 1-size map: got nil, wanted an error") } // Remove the previous limit. limitSet.Set(limits.NumberOfFiles, limits.Limit{maxFD, maxFD}, true) - if fds, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil { + if fds, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil { t.Fatalf("Adding an FD to a resized map: got %v, want nil", err) } else if len(fds) != 1 || fds[0] != 1 { t.Fatalf("Added an FD to a resized map: got %v, want {1}", fds) } - if err := fdTable.NewFDAt(ctx, 1, file, FDFlags{}); err != nil { + if err := fdTable.NewFDAtVFS2(ctx, 1, fd, FDFlags{}); err != nil { t.Fatalf("Replacing FD 1 via fdTable.NewFDAt(1, r, FDFlags{}): got %v, wanted nil", err) } - if err := fdTable.NewFDAt(ctx, maxFD+1, file, FDFlags{}); err == nil { + if err := fdTable.NewFDAtVFS2(ctx, maxFD+1, fd, FDFlags{}); err == nil { t.Fatalf("Using an FD that was too large via fdTable.NewFDAt(%v, r, FDFlags{}): got nil, wanted an error", maxFD+1) } - if ref, _ := fdTable.Get(1); ref == nil { - t.Fatalf("fdTable.Get(1): got nil, wanted %v", file) + if ref, _ := fdTable.GetVFS2(1); ref == nil { + t.Fatalf("fdTable.GetVFS2(1): got nil, wanted %v", fd) } - if ref, _ := fdTable.Get(2); ref != nil { - t.Fatalf("fdTable.Get(2): got a %v, wanted nil", ref) + if ref, _ := fdTable.GetVFS2(2); ref != nil { + t.Fatalf("fdTable.GetVFS2(2): got a %v, wanted nil", ref) } - ref, _ := fdTable.Remove(ctx, 1) + _, ref := fdTable.Remove(ctx, 1) if ref == nil { t.Fatalf("fdTable.Remove(1) for an existing FD: failed, want success") } ref.DecRef(ctx) - if ref, _ := fdTable.Remove(ctx, 1); ref != nil { + if _, ref := fdTable.Remove(ctx, 1); ref != nil { t.Fatalf("r.Remove(1) for a removed FD: got success, want failure") } }) } func TestDescriptorFlags(t *testing.T) { - runTest(t, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { - if err := fdTable.NewFDAt(ctx, 2, file, FDFlags{CloseOnExec: true}); err != nil { + runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { + if err := fdTable.NewFDAtVFS2(ctx, 2, fd, FDFlags{CloseOnExec: true}); err != nil { t.Fatalf("fdTable.NewFDAt(2, r, FDFlags{}): got %v, wanted nil", err) } - newFile, flags := fdTable.Get(2) + newFile, flags := fdTable.GetVFS2(2) if newFile == nil { - t.Fatalf("fdTable.Get(2): got a %v, wanted nil", newFile) + t.Fatalf("fdTable.GetVFS2(2): got a %v, wanted nil", newFile) } if !flags.CloseOnExec { @@ -182,15 +205,15 @@ func TestDescriptorFlags(t *testing.T) { func BenchmarkFDLookupAndDecRef(b *testing.B) { b.StopTimer() // Setup. - runTest(b, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { - fds, err := fdTable.NewFDs(ctx, 0, []*fs.File{file, file, file, file, file}, FDFlags{}) + runTest(b, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { + fds, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd, fd, fd, fd, fd}, FDFlags{}) if err != nil { b.Fatalf("fdTable.NewFDs: got %v, wanted nil", err) } b.StartTimer() // Benchmark. for i := 0; i < b.N; i++ { - tf, _ := fdTable.Get(fds[i%len(fds)]) + tf, _ := fdTable.GetVFS2(fds[i%len(fds)]) tf.DecRef(ctx) } }) @@ -199,8 +222,8 @@ func BenchmarkFDLookupAndDecRef(b *testing.B) { func BenchmarkFDLookupAndDecRefConcurrent(b *testing.B) { b.StopTimer() // Setup. - runTest(b, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { - fds, err := fdTable.NewFDs(ctx, 0, []*fs.File{file, file, file, file, file}, FDFlags{}) + runTest(b, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { + fds, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd, fd, fd, fd, fd}, FDFlags{}) if err != nil { b.Fatalf("fdTable.NewFDs: got %v, wanted nil", err) } @@ -218,7 +241,7 @@ func BenchmarkFDLookupAndDecRefConcurrent(b *testing.B) { go func() { defer wg.Done() for i := 0; i < each; i++ { - tf, _ := fdTable.Get(fds[i%len(fds)]) + tf, _ := fdTable.GetVFS2(fds[i%len(fds)]) tf.DecRef(ctx) } }() @@ -241,10 +264,10 @@ func TestSetFlagsForRange(t *testing.T) { } for _, test := range testCases { - runTest(t, func(ctx context.Context, fdTable *FDTable, file *fs.File, _ *limits.LimitSet) { + runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) { for i := 0; i < maxFD; i++ { - if _, err := fdTable.NewFDs(ctx, 0, []*fs.File{file}, FDFlags{}); err != nil { - t.Fatalf("testCase: %v\nfdTable.NewFDs(_, 0, %+v, FDFlags{}): %d, want: nil", test, []*fs.File{file}, err) + if _, err := fdTable.NewFDsVFS2(ctx, 0, []*vfs.FileDescription{fd}, FDFlags{}); err != nil { + t.Fatalf("testCase: %v\nfdTable.NewFDs(_, 0, %+v, FDFlags{}): %d, want: nil", test, []*vfs.FileDescription{fd}, err) } } @@ -259,9 +282,9 @@ func TestSetFlagsForRange(t *testing.T) { testRangeFlags := func(start int32, end int32, expected FDFlags) { for i := start; i <= end; i++ { - file, flags := fdTable.Get(i) + file, flags := fdTable.GetVFS2(i) if file == nil || flags != expected { - t.Fatalf("testCase: %v\nfdTable.Get(%d): (%v, %v), wanted (non-nil, %v)", test, i, file, flags, expected) + t.Fatalf("testCase: %v\nfdTable.GetVFS2(%d): (%v, %v), wanted (non-nil, %v)", test, i, file, flags, expected) } } } diff --git a/pkg/sentry/vfs/file_description_impl_util_test.go b/pkg/sentry/vfs/file_description_impl_util_test.go index ea8fb69b8..4c6554390 100644 --- a/pkg/sentry/vfs/file_description_impl_util_test.go +++ b/pkg/sentry/vfs/file_description_impl_util_test.go @@ -76,6 +76,7 @@ func (d *storeData) Write(ctx context.Context, _ *FileDescription, src usermem.I type testFD struct { fileDescription DynamicBytesFileDescriptionImpl + DentryMetadataFileDescriptionImpl data DynamicBytesSource } @@ -93,18 +94,6 @@ func newTestFD(ctx context.Context, vfsObj *VirtualFilesystem, statusFlags uint3 func (fd *testFD) Release(context.Context) { } -// SetStatusFlags implements FileDescriptionImpl.SetStatusFlags. -// Stat implements FileDescriptionImpl.Stat. -func (fd *testFD) Stat(ctx context.Context, opts StatOptions) (linux.Statx, error) { - // Note that Statx.Mask == 0 in the return value. - return linux.Statx{}, nil -} - -// SetStat implements FileDescriptionImpl.SetStat. -func (fd *testFD) SetStat(ctx context.Context, opts SetStatOptions) error { - return linuxerr.EPERM -} - func TestGenCountFD(t *testing.T) { ctx := contexttest.Context(t)