Track paths and provide a rename hook.

This change also adds extensive testing to the p9 package via mocks. The sanity
checks and type checks are moved from the gofer into the core package, where
they can be more easily validated.

PiperOrigin-RevId: 218296768
Change-Id: I4fc3c326e7bf1e0e140a454cbacbcc6fd617ab55

7 files changed, 139 insertions, 787 deletions

diff --git a/pkg/sentry/fs/anon/BUILD b/pkg/sentry/fs/anon/BUILD
index ff4ab850a..4bd912e95 100644
--- a/pkg/sentry/fs/anon/BUILD
+++ b/pkg/sentry/fs/anon/BUILD
@@ -1,7 +1,7 @@
-package(licenses = ["notice"])  # Apache 2.0
-
 load("//tools/go_stateify:defs.bzl", "go_library")
 
+package(licenses = ["notice"])  # Apache 2.0
+
 go_library(
     name = "anon",
     srcs = [
diff --git a/pkg/sentry/fs/gofer/BUILD b/pkg/sentry/fs/gofer/BUILD
index cef01829a..c9e531e40 100644
--- a/pkg/sentry/fs/gofer/BUILD
+++ b/pkg/sentry/fs/gofer/BUILD
@@ -56,14 +56,10 @@ go_test(
     srcs = ["gofer_test.go"],
     embed = [":gofer"],
     deps = [
-        "//pkg/log",
         "//pkg/p9",
         "//pkg/p9/p9test",
         "//pkg/sentry/context",
         "//pkg/sentry/context/contexttest",
         "//pkg/sentry/fs",
-        "//pkg/sentry/kernel/time",
-        "//pkg/sentry/usermem",
-        "//pkg/unet",
     ],
 )
diff --git a/pkg/sentry/fs/gofer/context_file.go b/pkg/sentry/fs/gofer/context_file.go
index a0265c2aa..455953237 100644
--- a/pkg/sentry/fs/gofer/context_file.go
+++ b/pkg/sentry/fs/gofer/context_file.go
@@ -58,13 +58,6 @@ func (c *contextFile) setAttr(ctx context.Context, valid p9.SetAttrMask, attr p9
 	return c.file.SetAttr(valid, attr)
 }
 
-func (c *contextFile) remove(ctx context.Context) error {
-	ctx.UninterruptibleSleepStart(false)
-	defer ctx.UninterruptibleSleepFinish(false)
-
-	return c.file.Remove()
-}
-
 func (c *contextFile) rename(ctx context.Context, directory contextFile, name string) error {
 	ctx.UninterruptibleSleepStart(false)
 	defer ctx.UninterruptibleSleepFinish(false)
diff --git a/pkg/sentry/fs/gofer/gofer_test.go b/pkg/sentry/fs/gofer/gofer_test.go
index 3190d1e18..b450778ca 100644
--- a/pkg/sentry/fs/gofer/gofer_test.go
+++ b/pkg/sentry/fs/gofer/gofer_test.go
@@ -16,110 +16,102 @@ package gofer
 
 import (
 	"fmt"
-	"io"
 	"syscall"
 	"testing"
 	"time"
 
-	"gvisor.googlesource.com/gvisor/pkg/log"
 	"gvisor.googlesource.com/gvisor/pkg/p9"
 	"gvisor.googlesource.com/gvisor/pkg/p9/p9test"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/context"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
-	ktime "gvisor.googlesource.com/gvisor/pkg/sentry/kernel/time"
-	"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
-	"gvisor.googlesource.com/gvisor/pkg/unet"
 )
 
-// goodMockFile returns a file that can be Walk'ed to and created.
-func goodMockFile(mode p9.FileMode, size uint64) *p9test.FileMock {
-	return &p9test.FileMock{
-		GetAttrMock: p9test.GetAttrMock{
-			Attr:  p9.Attr{Mode: mode, Size: size, RDev: 0},
-			Valid: p9.AttrMaskAll(),
-		},
-	}
-}
-
-func newClosedSocket() (*unet.Socket, error) {
-	fd, err := syscall.Socket(syscall.AF_UNIX, syscall.SOCK_STREAM, 0)
-	if err != nil {
-		return nil, err
-	}
-
-	s, err := unet.NewSocket(fd)
-	if err != nil {
-		syscall.Close(fd)
-		return nil, err
-	}
-
-	return s, s.Close()
-}
-
-// root returns a p9 file mock and an fs.InodeOperations created from that file.  Any
-// functions performed on fs.InodeOperations will use the p9 file mock.
-func root(ctx context.Context, cp cachePolicy, mode p9.FileMode, size uint64) (*p9test.FileMock, *fs.Inode, error) {
-	sock, err := newClosedSocket()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	// Construct a dummy session that we can destruct.
-	s := &session{
-		conn:        sock,
-		mounter:     fs.RootOwner,
-		cachePolicy: cp,
-		client:      &p9.Client{},
-	}
-
-	rootFile := goodMockFile(mode, size)
-	sattr, rootInodeOperations := newInodeOperations(ctx, s, contextFile{file: rootFile}, p9.QID{}, rootFile.GetAttrMock.Valid, rootFile.GetAttrMock.Attr, false /* socket */)
-	m := fs.NewMountSource(s, &filesystem{}, fs.MountSourceFlags{})
-	return rootFile, fs.NewInode(rootInodeOperations, m, sattr), nil
+// rootTest runs a test with a p9 mock and an fs.InodeOperations created from
+// the attached root directory. The root file will be closed and client
+// disconnected, but additional files must be closed manually.
+func rootTest(t *testing.T, name string, cp cachePolicy, fn func(context.Context, *p9test.Harness, *p9test.Mock, *fs.Inode)) {
+	t.Run(name, func(t *testing.T) {
+		h, c := p9test.NewHarness(t)
+		defer h.Finish()
+
+		// Create a new root. Note that we pass an empty, but non-nil
+		// map here. This allows tests to extend the root children
+		// dynamically.
+		root := h.NewDirectory(map[string]p9test.Generator{})(nil)
+
+		// Return this as the root.
+		h.Attacher.EXPECT().Attach().Return(root, nil).Times(1)
+
+		// ... and open via the client.
+		rootFile, err := c.Attach("/")
+		if err != nil {
+			t.Fatalf("unable to attach: %v", err)
+		}
+		defer rootFile.Close()
+
+		// Wrap an a session.
+		s := &session{
+			mounter:     fs.RootOwner,
+			cachePolicy: cp,
+			client:      c,
+		}
+
+		// ... and an INode, with only the mode being explicitly valid for now.
+		ctx := contexttest.Context(t)
+		sattr, rootInodeOperations := newInodeOperations(ctx, s, contextFile{
+			file: rootFile,
+		}, root.QID, p9.AttrMaskAll(), root.Attr, false /* socket */)
+		m := fs.NewMountSource(s, &filesystem{}, fs.MountSourceFlags{})
+		rootInode := fs.NewInode(rootInodeOperations, m, sattr)
+
+		// Ensure that the cache is fully invalidated, so that any
+		// close actions actually take place before the full harness is
+		// torn down.
+		defer m.FlushDirentRefs()
+
+		// Execute the test.
+		fn(ctx, h, root, rootInode)
+	})
 }
 
 func TestLookup(t *testing.T) {
-	// Test parameters.
 	type lookupTest struct {
 		// Name of the test.
 		name string
 
-		// Function input parameters.
-		fileName string
-
 		// Expected return value.
 		want error
 	}
 
 	tests := []lookupTest{
 		{
-			name:     "mock Walk passes (function succeeds)",
-			fileName: "ppp",
-			want:     nil,
+			name: "mock Walk passes (function succeeds)",
+			want: nil,
 		},
 		{
-			name:     "mock Walk fails (function fails)",
-			fileName: "ppp",
-			want:     syscall.ENOENT,
+			name: "mock Walk fails (function fails)",
+			want: syscall.ENOENT,
 		},
 	}
 
-	ctx := contexttest.Context(t)
+	const file = "file" // The walked target file.
+
 	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, cacheNone, p9.PermissionsMask, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
+		rootTest(t, test.name, cacheNone, func(ctx context.Context, h *p9test.Harness, rootFile *p9test.Mock, rootInode *fs.Inode) {
+			// Setup the appropriate result.
+			rootFile.WalkCallback = func() error {
+				return test.want
+			}
+			if test.want == nil {
+				// Set the contents of the root. We expect a
+				// normal file generator for ppp above. This is
+				// overriden by setting WalkErr in the mock.
+				rootFile.AddChild(file, h.NewFile())
 			}
-
-			rootFile.WalkGetAttrMock.QIDs = []p9.QID{{}}
-			rootFile.WalkGetAttrMock.Err = test.want
-			rootFile.WalkGetAttrMock.File = goodMockFile(p9.PermissionsMask, 0)
 
 			// Call function.
-			dirent, err := rootInode.Lookup(ctx, test.fileName)
+			dirent, err := rootInode.Lookup(ctx, file)
 
 			// Unwrap the InodeOperations.
 			var newInodeOperations fs.InodeOperations
@@ -138,19 +130,12 @@ func TestLookup(t *testing.T) {
 			if err == nil && newInodeOperations == nil {
 				t.Errorf("Lookup got non-nil err and non-nil node, wanted at least one non-nil")
 			}
-
-			// Check mock parameters.
-			if !rootFile.WalkGetAttrMock.Called {
-				t.Errorf("GetAttr not called; error: %v", err)
-			} else if rootFile.WalkGetAttrMock.Names[0] != test.fileName {
-				t.Errorf("file name not set")
-			}
 		})
 	}
 }
 
 func TestRevalidation(t *testing.T) {
-	tests := []struct {
+	type revalidationTest struct {
 		cachePolicy cachePolicy
 
 		// Whether dirent should be reloaded before any modifications.
@@ -167,7 +152,9 @@ func TestRevalidation(t *testing.T) {
 		// Whether dirent should be reloaded after the remote has
 		// removed the file.
 		postRemovalWantReload bool
-	}{
+	}
+
+	tests := []revalidationTest{
 		{
 			// Policy cacheNone causes Revalidate to always return
 			// true.
@@ -208,67 +195,83 @@ func TestRevalidation(t *testing.T) {
 		},
 	}
 
-	ctx := contexttest.Context(t)
+	const file = "file" // The file walked below.
+
 	for _, test := range tests {
 		name := fmt.Sprintf("cachepolicy=%s", test.cachePolicy)
-		t.Run(name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, test.cachePolicy, p9.ModeDirectory|p9.PermissionsMask, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-
+		rootTest(t, name, test.cachePolicy, func(ctx context.Context, h *p9test.Harness, rootFile *p9test.Mock, rootInode *fs.Inode) {
+			// Wrap in a dirent object.
 			rootDir := fs.NewDirent(rootInode, "root")
 
-			// Create a mock file that we will walk to from the root.
-			const (
-				name = "foo"
-				mode = p9.PermissionsMask
-			)
-			file := goodMockFile(mode, 0)
-			file.GetAttrMock.Valid = p9.AttrMaskAll()
-
-			// Tell the root mock how to walk to this file.
-			rootFile.WalkGetAttrMock.QIDs = []p9.QID{{}}
-			rootFile.WalkGetAttrMock.File = file
-			rootFile.WalkGetAttrMock.Attr = file.GetAttrMock.Attr
-			rootFile.WalkGetAttrMock.Valid = file.GetAttrMock.Valid
+			// Create a mock file a child of the root. We save when
+			// this is generated, so that when the time changed, we
+			// can update the original entry.
+			var origMocks []*p9test.Mock
+			rootFile.AddChild(file, func(parent *p9test.Mock) *p9test.Mock {
+				// Regular a regular file that has a consistent
+				// path number. This might be used by
+				// validation so we don't change it.
+				m := h.NewMock(parent, 0, p9.Attr{
+					Mode: p9.ModeRegular,
+				})
+				origMocks = append(origMocks, m)
+				return m
+			})
 
 			// Do the walk.
-			dirent, err := rootDir.Walk(ctx, rootDir, name)
+			dirent, err := rootDir.Walk(ctx, rootDir, file)
 			if err != nil {
-				t.Fatalf("Lookup(%q) failed: %v", name, err)
+				t.Fatalf("Lookup failed: %v", err)
 			}
 
-			// Walk again. Depending on the cache policy, we may get a new
-			// dirent.
-			newDirent, err := rootDir.Walk(ctx, rootDir, name)
+			// We must release the dirent, of the test will fail
+			// with a reference leak. This is tracked by p9test.
+			defer dirent.DecRef()
+
+			// Walk again. Depending on the cache policy, we may
+			// get a new dirent.
+			newDirent, err := rootDir.Walk(ctx, rootDir, file)
 			if err != nil {
-				t.Fatalf("Lookup(%q) failed: %v", name, err)
+				t.Fatalf("Lookup failed: %v", err)
 			}
 			if test.preModificationWantReload && dirent == newDirent {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got old dirent %v, wanted a new dirent", name, test.cachePolicy, dirent)
+				t.Errorf("Lookup with cachePolicy=%s got old dirent %+v, wanted a new dirent", test.cachePolicy, dirent)
 			}
 			if !test.preModificationWantReload && dirent != newDirent {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got new dirent %v, wanted old dirent %v", name, test.cachePolicy, newDirent, dirent)
+				t.Errorf("Lookup with cachePolicy=%s got new dirent %+v, wanted old dirent %+v", test.cachePolicy, newDirent, dirent)
 			}
+			newDirent.DecRef() // See above.
 
-			// Modify the underlying mocked file's modification time.
+			// Modify the underlying mocked file's modification
+			// time for the next walk that occurs.
 			nowSeconds := time.Now().Unix()
-			rootFile.WalkGetAttrMock.Attr.MTimeSeconds = uint64(nowSeconds)
-			file.GetAttrMock.Attr.MTimeSeconds = uint64(nowSeconds)
+			rootFile.AddChild(file, func(parent *p9test.Mock) *p9test.Mock {
+				// Ensure that the path is the same as above,
+				// but we change only the modification time of
+				// the file.
+				return h.NewMock(parent, 0, p9.Attr{
+					Mode:         p9.ModeRegular,
+					MTimeSeconds: uint64(nowSeconds),
+				})
+			})
+
+			// We also modify the original time, so that GetAttr
+			// behaves as expected for the caching case.
+			for _, m := range origMocks {
+				m.Attr.MTimeSeconds = uint64(nowSeconds)
+			}
 
-			// Walk again. Depending on the cache policy, we may get a new
-			// dirent.
-			newDirent, err = rootDir.Walk(ctx, rootDir, name)
+			// Walk again. Depending on the cache policy, we may
+			// get a new dirent.
+			newDirent, err = rootDir.Walk(ctx, rootDir, file)
 			if err != nil {
-				t.Fatalf("Lookup(%q) failed: %v", name, err)
+				t.Fatalf("Lookup failed: %v", err)
 			}
 			if test.postModificationWantReload && dirent == newDirent {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got old dirent %v, wanted a new dirent", name, test.cachePolicy, dirent)
+				t.Errorf("Lookup with cachePolicy=%s got old dirent, wanted a new dirent", test.cachePolicy)
 			}
 			if !test.postModificationWantReload && dirent != newDirent {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got new dirent %v, wanted old dirent %v", name, test.cachePolicy, newDirent, dirent)
+				t.Errorf("Lookup with cachePolicy=%s got new dirent, wanted old dirent", test.cachePolicy)
 			}
 			uattrs, err := newDirent.Inode.UnstableAttr(ctx)
 			if err != nil {
@@ -276,660 +279,25 @@ func TestRevalidation(t *testing.T) {
 			}
 			gotModTimeSeconds := uattrs.ModificationTime.Seconds()
 			if test.postModificationWantUpdatedAttrs && gotModTimeSeconds != nowSeconds {
-				t.Fatalf("Lookup(%q) with cachePolicy=%s got new modification time %v, wanted %v", name, test.cachePolicy, gotModTimeSeconds, nowSeconds)
+				t.Fatalf("Lookup with cachePolicy=%s got new modification time %v, wanted %v", test.cachePolicy, gotModTimeSeconds, nowSeconds)
 			}
+			newDirent.DecRef() // See above.
 
-			// Make WalkGetAttr return ENOENT. This simulates
-			// removing the file from the remote fs.
-			rootFile.WalkGetAttrMock = p9test.WalkGetAttrMock{
-				Err: syscall.ENOENT,
-			}
+			// Remove the file from the remote fs, subsequent walks
+			// should now fail to find anything.
+			rootFile.RemoveChild(file)
 
 			// Walk again. Depending on the cache policy, we may
 			// get ENOENT.
-			newDirent, err = rootDir.Walk(ctx, rootDir, name)
+			newDirent, err = rootDir.Walk(ctx, rootDir, file)
 			if test.postRemovalWantReload && err == nil {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got nil error, wanted ENOENT", name, test.cachePolicy)
+				t.Errorf("Lookup with cachePolicy=%s got nil error, wanted ENOENT", test.cachePolicy)
 			}
 			if !test.postRemovalWantReload && (err != nil || dirent != newDirent) {
-				t.Errorf("Lookup(%q) with cachePolicy=%s got new dirent %v and error %v, wanted old dirent %v and nil error", name, test.cachePolicy, newDirent, err, dirent)
-			}
-		})
-	}
-}
-
-func TestSetTimestamps(t *testing.T) {
-	// Test parameters.
-	type setTimestampsTest struct {
-		// Name of the test.
-		name string
-
-		// Function input parameters.
-		ts fs.TimeSpec
-	}
-
-	ctx := contexttest.Context(t)
-	now := ktime.NowFromContext(ctx)
-	tests := []setTimestampsTest{
-		{
-			name: "mock SetAttr passes (function succeeds)",
-			ts: fs.TimeSpec{
-				ATime: now,
-				MTime: now,
-			},
-		},
-		{
-			name: "mock SetAttr passes, times are 0 (function succeeds)",
-			ts:   fs.TimeSpec{},
-		},
-		{
-			name: "mock SetAttr passes, times are 0 and not system time (function succeeds)",
-			ts: fs.TimeSpec{
-				ATimeSetSystemTime: false,
-				MTimeSetSystemTime: false,
-			},
-		},
-		{
-			name: "mock SetAttr passes, times are set to system time (function succeeds)",
-			ts: fs.TimeSpec{
-				ATimeSetSystemTime: true,
-				MTimeSetSystemTime: true,
-			},
-		},
-		{
-			name: "mock SetAttr passes, times are omitted (function succeeds)",
-			ts: fs.TimeSpec{
-				ATimeOmit: true,
-				MTimeOmit: true,
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, cacheNone, p9.PermissionsMask, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-
-			// Call function.
-			err = rootInode.SetTimestamps(ctx, nil /* Dirent */, test.ts)
-
-			// Check return values.
-			if err != nil {
-				t.Errorf("SetTimestamps failed: got error %v, want nil", err)
-			}
-
-			// Check mock parameters.
-			if !(test.ts.ATimeOmit && test.ts.MTimeOmit) && !rootFile.SetAttrMock.Called {
-				t.Errorf("TestSetTimestamps failed: SetAttr not called")
-				return
-			}
-
-			// Check what was passed to the mock function.
-			attr := rootFile.SetAttrMock.Attr
-			atimeGiven := ktime.FromUnix(int64(attr.ATimeSeconds), int64(attr.ATimeNanoSeconds))
-			if test.ts.ATimeOmit {
-				if rootFile.SetAttrMock.Valid.ATime {
-					t.Errorf("ATime got set true in mask, wanted false")
-				}
-			} else {
-				if got, want := rootFile.SetAttrMock.Valid.ATimeNotSystemTime, !test.ts.ATimeSetSystemTime; got != want {
-					t.Errorf("got ATimeNotSystemTime %v, want %v", got, want)
-				}
-				if !test.ts.ATimeSetSystemTime && !test.ts.ATime.Equal(atimeGiven) {
-					t.Errorf("ATime got %v, want %v", atimeGiven, test.ts.ATime)
-				}
-			}
-
-			mtimeGiven := ktime.FromUnix(int64(attr.MTimeSeconds), int64(attr.MTimeNanoSeconds))
-			if test.ts.MTimeOmit {
-				if rootFile.SetAttrMock.Valid.MTime {
-					t.Errorf("MTime got set true in mask, wanted false")
-				}
-			} else {
-				if got, want := rootFile.SetAttrMock.Valid.MTimeNotSystemTime, !test.ts.MTimeSetSystemTime; got != want {
-					t.Errorf("got MTimeNotSystemTime %v, want %v", got, want)
-				}
-				if !test.ts.MTimeSetSystemTime && !test.ts.MTime.Equal(mtimeGiven) {
-					t.Errorf("MTime got %v, want %v", mtimeGiven, test.ts.MTime)
-				}
-			}
-		})
-	}
-}
-
-func TestSetPermissions(t *testing.T) {
-	// Test parameters.
-	type setPermissionsTest struct {
-		// Name of the test.
-		name string
-
-		// SetPermissions input parameters.
-		perms fs.FilePermissions
-
-		// Error that SetAttr mock should return.
-		setAttrErr error
-
-		// Expected return value.
-		want bool
-	}
-
-	tests := []setPermissionsTest{
-		{
-			name:       "SetAttr mock succeeds (function succeeds)",
-			perms:      fs.FilePermissions{User: fs.PermMask{Read: true, Write: true, Execute: true}},
-			want:       true,
-			setAttrErr: nil,
-		},
-		{
-			name:       "SetAttr mock fails (function fails)",
-			perms:      fs.FilePermissions{User: fs.PermMask{Read: true, Write: true}},
-			want:       false,
-			setAttrErr: syscall.ENOENT,
-		},
-	}
-
-	ctx := contexttest.Context(t)
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, cacheNone, 0, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-			rootFile.SetAttrMock.Err = test.setAttrErr
-
-			ok := rootInode.SetPermissions(ctx, nil /* Dirent */, test.perms)
-
-			// Check return value.
-			if ok != test.want {
-				t.Errorf("SetPermissions got %v, want %v", ok, test.want)
-			}
-
-			// Check mock parameters.
-			pattr := rootFile.SetAttrMock.Attr
-			if !rootFile.SetAttrMock.Called {
-				t.Errorf("SetAttr not called")
-				return
-			}
-			if !rootFile.SetAttrMock.Valid.Permissions {
-				t.Errorf("SetAttr did not get right request (got false, expected SetAttrMask.Permissions true)")
-			}
-			if got := fs.FilePermsFromP9(pattr.Permissions); got != test.perms {
-				t.Errorf("SetAttr did not get right permissions -- got %v, want %v", got, test.perms)
-			}
-		})
-	}
-}
-
-func TestClose(t *testing.T) {
-	ctx := contexttest.Context(t)
-	// Set up mock.
-	rootFile, rootInode, err := root(ctx, cacheNone, p9.PermissionsMask, 0)
-	if err != nil {
-		t.Fatalf("error creating root: %v", err)
-	}
-
-	// Call function.
-	rootInode.InodeOperations.Release(ctx)
-
-	// Check mock parameters.
-	if !rootFile.CloseMock.Called {
-		t.Errorf("TestClose failed: Close not called")
-	}
-}
-
-func TestRename(t *testing.T) {
-	// Test parameters.
-	type renameTest struct {
-		// Name of the test.
-		name string
-
-		// Input parameters.
-		newParent *fs.Inode
-		newName   string
-
-		// Rename mock parameters.
-		renameErr    error
-		renameCalled bool
-
-		// Error want to return given the parameters. (Same as what
-		// we expect and tell rename to return.)
-		want error
-	}
-	ctx := contexttest.Context(t)
-	rootFile, rootInode, err := root(ctx, cacheNone, p9.PermissionsMask, 0)
-	if err != nil {
-		t.Fatalf("error creating root: %v", err)
-	}
-
-	tests := []renameTest{
-		{
-			name:         "mock Rename succeeds (function succeeds)",
-			newParent:    rootInode,
-			newName:      "foo2",
-			want:         nil,
-			renameErr:    nil,
-			renameCalled: true,
-		},
-		{
-			name:         "mock Rename fails (function fails)",
-			newParent:    rootInode,
-			newName:      "foo2",
-			want:         syscall.ENOENT,
-			renameErr:    syscall.ENOENT,
-			renameCalled: true,
-		},
-		{
-			name:         "newParent is not inodeOperations but should be (function fails)",
-			newParent:    fs.NewMockInode(ctx, fs.NewMockMountSource(nil), fs.StableAttr{Type: fs.Directory}),
-			newName:      "foo2",
-			want:         syscall.EXDEV,
-			renameErr:    nil,
-			renameCalled: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			mockFile := goodMockFile(p9.PermissionsMask, 0)
-			rootFile.WalkGetAttrMock.QIDs = []p9.QID{{}}
-			rootFile.WalkGetAttrMock.File = mockFile
-
-			dirent, err := rootInode.Lookup(ctx, "foo")
-			if err != nil {
-				t.Fatalf("root.Walk failed: %v", err)
-			}
-			mockFile.RenameMock.Err = test.renameErr
-			mockFile.RenameMock.Called = false
-
-			// Use a dummy oldParent to acquire write access to that directory.
-			oldParent := &inodeOperations{
-				readdirCache: fs.NewSortedDentryMap(nil),
-			}
-			oldInode := fs.NewInode(oldParent, fs.NewMockMountSource(nil), fs.StableAttr{Type: fs.Directory})
-
-			// Call function.
-			err = dirent.Inode.InodeOperations.Rename(ctx, oldInode, "", test.newParent, test.newName)
-
-			// Check return value.
-			if err != test.want {
-				t.Errorf("Rename got %v, want %v", err, test.want)
-			}
-
-			// Check mock parameters.
-			if got, want := mockFile.RenameMock.Called, test.renameCalled; got != want {
-				t.Errorf("renameCalled got %v want %v", got, want)
-			}
-		})
-	}
-}
-
-// This file is read from in TestPreadv.
-type readAtFileFake struct {
-	p9test.FileMock
-
-	// Parameters for faking ReadAt.
-	FileLength int
-	Err        error
-	ChunkSize  int
-	Called     bool
-	LengthRead int
-}
-
-func (r *readAtFileFake) ReadAt(p []byte, offset uint64) (int, error) {
-	r.Called = true
-	log.Warningf("ReadAt fake: length read so far = %d, len(p) = %d, offset = %d", r.LengthRead, len(p), offset)
-	if int(offset) != r.LengthRead {
-		return 0, fmt.Errorf("offset got %d; expected %d", offset, r.LengthRead)
-	}
-
-	if r.Err != nil {
-		return 0, r.Err
-	}
-
-	if r.LengthRead >= r.FileLength {
-		return 0, io.EOF
-	}
-
-	// Read at most ChunkSize and read at most what's left in the file.
-	toBeRead := len(p)
-	if r.LengthRead+toBeRead >= r.FileLength {
-		toBeRead = r.FileLength - int(offset)
-	}
-	if toBeRead > r.ChunkSize {
-		toBeRead = r.ChunkSize
-	}
-
-	r.LengthRead += toBeRead
-	if r.LengthRead == r.FileLength {
-		return toBeRead, io.EOF
-	}
-	return toBeRead, nil
-}
-
-func TestPreadv(t *testing.T) {
-	// Test parameters.
-	type preadvTest struct {
-		// Name of the test.
-		name string
-
-		// Mock parameters
-		mode p9.FileMode
-
-		// Buffer to read into.
-		buffer    [512]byte
-		sliceSize int
-
-		// How much readAt returns at a time.
-		chunkSize int
-
-		// Whether or not we expect ReadAt to be called.
-		readAtCalled bool
-		readAtErr    error
-
-		// Expected return values.
-		want error
-	}
-
-	tests := []preadvTest{
-		{
-			name:         "fake ReadAt succeeds, 512 bytes requested, 512 byte chunks (function succeeds)",
-			want:         nil,
-			readAtErr:    nil,
-			mode:         p9.PermissionsMask,
-			readAtCalled: true,
-			sliceSize:    512,
-			chunkSize:    512,
-		},
-		{
-			name:         "fake ReadAt succeeds, 512 bytes requested, 200 byte chunks (function succeeds)",
-			want:         nil,
-			readAtErr:    nil,
-			mode:         p9.PermissionsMask,
-			readAtCalled: true,
-			sliceSize:    512,
-			chunkSize:    200,
-		},
-		{
-			name:         "fake ReadAt succeeds, 0 bytes requested (function succeeds)",
-			want:         nil,
-			readAtErr:    nil,
-			mode:         p9.PermissionsMask,
-			readAtCalled: false,
-			sliceSize:    0,
-			chunkSize:    100,
-		},
-		{
-			name:         "fake ReadAt returns 0 bytes and EOF (function fails)",
-			want:         io.EOF,
-			readAtErr:    io.EOF,
-			mode:         p9.PermissionsMask,
-			readAtCalled: true,
-			sliceSize:    512,
-			chunkSize:    512,
-		},
-	}
-
-	ctx := contexttest.Context(t)
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, cacheNone, test.mode, 1024)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-
-			// Set up the read buffer.
-			dst := usermem.BytesIOSequence(test.buffer[:test.sliceSize])
-
-			// This file will be read from.
-			openFile := &readAtFileFake{
-				Err:        test.readAtErr,
-				FileLength: test.sliceSize,
-				ChunkSize:  test.chunkSize,
-			}
-			rootFile.WalkGetAttrMock.File = openFile
-			rootFile.WalkGetAttrMock.Attr.Mode = test.mode
-			rootFile.WalkGetAttrMock.Valid.Mode = true
-
-			f := NewFile(
-				ctx,
-				fs.NewDirent(rootInode, ""),
-				"",
-				fs.FileFlags{Read: true},
-				rootInode.InodeOperations.(*inodeOperations),
-				&handles{File: contextFile{file: openFile}},
-			)
-
-			// Call function.
-			_, err = f.Preadv(ctx, dst, 0)
-
-			// Check return value.
-			if err != test.want {
-				t.Errorf("Preadv got %v, want %v", err, test.want)
-			}
-
-			// Check mock parameters.
-			if test.readAtCalled != openFile.Called {
-				t.Errorf("ReadAt called: %v, but expected opposite", openFile.Called)
-			}
-		})
-	}
-}
-
-func TestReadlink(t *testing.T) {
-	// Test parameters.
-	type readlinkTest struct {
-		// Name of the test.
-		name string
-
-		// Mock parameters
-		mode p9.FileMode
-
-		// Whether or not we expect ReadAt to be called and what error
-		// it shall return.
-		readlinkCalled bool
-		readlinkErr    error
-
-		// Expected return values.
-		want error
-	}
-
-	tests := []readlinkTest{
-		{
-			name:           "file is not symlink (function fails)",
-			want:           syscall.ENOLINK,
-			mode:           p9.PermissionsMask,
-			readlinkCalled: false,
-			readlinkErr:    nil,
-		},
-		{
-			name:           "mock Readlink succeeds (function succeeds)",
-			want:           nil,
-			mode:           p9.PermissionsMask | p9.ModeSymlink,
-			readlinkCalled: true,
-			readlinkErr:    nil,
-		},
-		{
-			name:           "mock Readlink fails (function fails)",
-			want:           syscall.ENOENT,
-			mode:           p9.PermissionsMask | p9.ModeSymlink,
-			readlinkCalled: true,
-			readlinkErr:    syscall.ENOENT,
-		},
-	}
-
-	ctx := contexttest.Context(t)
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			rootFile, rootInode, err := root(ctx, cacheNone, test.mode, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-
-			openFile := goodMockFile(test.mode, 0)
-			rootFile.WalkMock.File = openFile
-			rootFile.ReadlinkMock.Err = test.readlinkErr
-
-			// Call function.
-			_, err = rootInode.Readlink(ctx)
-
-			// Check return value.
-			if err != test.want {
-				t.Errorf("Readlink got %v, want %v", err, test.want)
-			}
-
-			// Check mock parameters.
-			if test.readlinkCalled && !rootFile.ReadlinkMock.Called {
-				t.Errorf("Readlink not called")
-			}
-		})
-	}
-}
-
-// This file is write from in TestPwritev.
-type writeAtFileFake struct {
-	p9test.FileMock
-
-	// Parameters for faking WriteAt.
-	Err           error
-	ChunkSize     int
-	Called        bool
-	LengthWritten int
-}
-
-func (r *writeAtFileFake) WriteAt(p []byte, offset uint64) (int, error) {
-	r.Called = true
-	log.Warningf("WriteAt fake: length written so far = %d, len(p) = %d, offset = %d", r.LengthWritten, len(p), offset)
-	if int(offset) != r.LengthWritten {
-		return 0, fmt.Errorf("offset got %d; want %d", offset, r.LengthWritten)
-	}
-
-	if r.Err != nil {
-		return 0, r.Err
-	}
-
-	// Write at most ChunkSize.
-	toBeWritten := len(p)
-	if toBeWritten > r.ChunkSize {
-		toBeWritten = r.ChunkSize
-	}
-	r.LengthWritten += toBeWritten
-	return toBeWritten, nil
-}
-
-func TestPwritev(t *testing.T) {
-	// Test parameters.
-	type pwritevTest struct {
-		// Name of the test.
-		name string
-
-		// Mock parameters
-		mode p9.FileMode
-
-		allowWrite bool
-
-		// Buffer to write into.
-		buffer    [512]byte
-		sliceSize int
-		chunkSize int
-
-		// Whether or not we expect writeAt to be called.
-		writeAtCalled bool
-		writeAtErr    error
-
-		// Expected return values.
-		want error
-	}
-
-	tests := []pwritevTest{
-		{
-			name:          "fake writeAt succeeds, one chunk (function succeeds)",
-			want:          nil,
-			writeAtErr:    nil,
-			mode:          p9.PermissionsMask,
-			allowWrite:    true,
-			writeAtCalled: true,
-			sliceSize:     512,
-			chunkSize:     512,
-		},
-		{
-			name:          "fake writeAt fails, short write (function fails)",
-			want:          io.ErrShortWrite,
-			writeAtErr:    nil,
-			mode:          p9.PermissionsMask,
-			allowWrite:    true,
-			writeAtCalled: true,
-			sliceSize:     512,
-			chunkSize:     200,
-		},
-		{
-			name:          "fake writeAt succeeds, len 0 (function succeeds)",
-			want:          nil,
-			writeAtErr:    nil,
-			mode:          p9.PermissionsMask,
-			allowWrite:    true,
-			writeAtCalled: false,
-			sliceSize:     0,
-			chunkSize:     0,
-		},
-		{
-			name:          "writeAt can still write despite file permissions read only (function succeeds)",
-			want:          nil,
-			writeAtErr:    nil,
-			mode:          p9.PermissionsMask,
-			allowWrite:    false,
-			writeAtCalled: true,
-			sliceSize:     512,
-			chunkSize:     512,
-		},
-	}
-
-	ctx := contexttest.Context(t)
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Set up mock.
-			_, rootInode, err := root(ctx, cacheNone, test.mode, 0)
-			if err != nil {
-				t.Fatalf("error creating root: %v", err)
-			}
-
-			src := usermem.BytesIOSequence(test.buffer[:test.sliceSize])
-
-			// This is the file that will be used for writing.
-			openFile := &writeAtFileFake{
-				Err:       test.writeAtErr,
-				ChunkSize: test.chunkSize,
-			}
-
-			f := NewFile(
-				ctx,
-				fs.NewDirent(rootInode, ""),
-				"",
-				fs.FileFlags{Write: true},
-				rootInode.InodeOperations.(*inodeOperations),
-				&handles{File: contextFile{file: openFile}},
-			)
-
-			// Call function.
-			_, err = f.Pwritev(ctx, src, 0)
-
-			// Check return value.
-			if err != test.want {
-				t.Errorf("Pwritev got %v, want %v", err, test.want)
-			}
-
-			// Check mock parameters.
-			if test.writeAtCalled != openFile.Called {
-				t.Errorf("WriteAt called: %v, but expected opposite", openFile.Called)
-				return
+				t.Errorf("Lookup with cachePolicy=%s got new dirent and error %v, wanted old dirent and nil error", test.cachePolicy, err)
 			}
-			if openFile.Called && test.writeAtErr != nil && openFile.LengthWritten != test.sliceSize {
-				t.Errorf("wrote %d bytes, expected %d bytes written", openFile.LengthWritten, test.sliceSize)
+			if err == nil {
+				newDirent.DecRef() // See above.
 			}
 		})
 	}
diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go
index 7552216f3..f76a83cd9 100644
--- a/pkg/sentry/fs/gofer/session.go
+++ b/pkg/sentry/fs/gofer/session.go
@@ -91,10 +91,6 @@ func (e *endpointMaps) get(key device.MultiDeviceKey) transport.BoundEndpoint {
 type session struct {
 	refs.AtomicRefCount
 
-	// conn is a unet.Socket that wraps the readFD/writeFD mount option,
-	// see fs/gofer/fs.go.
-	conn *unet.Socket `state:"nosave"`
-
 	// msize is the value of the msize mount option, see fs/gofer/fs.go.
 	msize uint32 `state:"wait"`
 
@@ -142,7 +138,7 @@ type session struct {
 
 // Destroy tears down the session.
 func (s *session) Destroy() {
-	s.conn.Close()
+	s.client.Close()
 }
 
 // Revalidate implements MountSource.Revalidate.
@@ -235,7 +231,6 @@ func Root(ctx context.Context, dev string, filesystem fs.Filesystem, superBlockF
 	// Construct the session.
 	s := &session{
 		connID:          dev,
-		conn:            conn,
 		msize:           o.msize,
 		version:         o.version,
 		cachePolicy:     o.policy,
@@ -252,7 +247,7 @@ func Root(ctx context.Context, dev string, filesystem fs.Filesystem, superBlockF
 	m := fs.NewMountSource(s, filesystem, superBlockFlags)
 
 	// Send the Tversion request.
-	s.client, err = p9.NewClient(s.conn, s.msize, s.version)
+	s.client, err = p9.NewClient(conn, s.msize, s.version)
 	if err != nil {
 		// Drop our reference on the session, it needs to be torn down.
 		s.DecRef()
diff --git a/pkg/sentry/fs/gofer/session_state.go b/pkg/sentry/fs/gofer/session_state.go
index f657135fc..d9fd7a221 100644
--- a/pkg/sentry/fs/gofer/session_state.go
+++ b/pkg/sentry/fs/gofer/session_state.go
@@ -84,13 +84,13 @@ func (s *session) afterLoad() {
 	}
 
 	// Manually restore the connection.
-	s.conn, err = unet.NewSocket(opts.fd)
+	conn, err := unet.NewSocket(opts.fd)
 	if err != nil {
 		panic(fmt.Sprintf("failed to create Socket for FD %d: %v", opts.fd, err))
 	}
 
 	// Manually restore the client.
-	s.client, err = p9.NewClient(s.conn, s.msize, s.version)
+	s.client, err = p9.NewClient(conn, s.msize, s.version)
 	if err != nil {
 		panic(fmt.Sprintf("failed to connect client to server: %v", err))
 	}
diff --git a/pkg/sentry/fs/proc/device/BUILD b/pkg/sentry/fs/proc/device/BUILD
index 34582f275..ff7dacf07 100644
--- a/pkg/sentry/fs/proc/device/BUILD
+++ b/pkg/sentry/fs/proc/device/BUILD
@@ -1,7 +1,7 @@
-package(licenses = ["notice"])  # Apache 2.0
-
 load("//tools/go_stateify:defs.bzl", "go_library")
 
+package(licenses = ["notice"])  # Apache 2.0
+
 go_library(
     name = "device",
     srcs = ["device.go"],