gofer: Support restore of deleted directories whose original path is occupied.

It is possible that the application can open an FD to a directory, delete the
directory, create another directory at that path and then checkpoint the
container. In such a scenario, our gofer restore logic will fail with EEXIST
because a directory already exists on the host at that location when we try to
recreate it.

Use a temporary filename to recreate the directory in such a scenario. This
directory is deleted again anyways.

This is similar to e3c4c4c ("gofer: Support restore of deleted files whose
original path is occupied.")

In addition to this, this change also makes the following bug fixes:
- Adds a random suffix to the temporary filename used to recreate the file/dir.
  This helps if multiple (different) deleted open directories exist at the same
  position.
- Change dentry.name for the duration of the restoreDeleted*() function. Note
  that dentry.restoreFile() use dentry.name to re-open handles and control FD.
  Hence setting the correct dentry.name is crucial for the handles to point to
  the correct file.
- Added a syscall test, which when run in save/restore mode, tests these code
  paths for handling open deleted files/directories.

Updates #11425

PiperOrigin-RevId: 775406486

Author: ayushr2

pkg/sentry/fsimpl/gofer/BUILD

@@ -93,6 +93,7 @@ go_library(
         "//pkg/marshal",
         "//pkg/marshal/primitive",
         "//pkg/metric",
+        "//pkg/rand",
         "//pkg/refs",
         "//pkg/safemem",
         "//pkg/sentry/fsimpl/host",

pkg/sentry/fsimpl/gofer/save_restore.go

@@ -16,6 +16,7 @@ package gofer
 
 import (
 	goContext "context"
+	"encoding/hex"
 	"fmt"
 	"io"
 
@@ -27,6 +28,7 @@ import (
 	"gvisor.dev/gvisor/pkg/fdnotifier"
 	"gvisor.dev/gvisor/pkg/hostarch"
 	"gvisor.dev/gvisor/pkg/log"
+	"gvisor.dev/gvisor/pkg/rand"
 	"gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/safemem"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
@@ -327,7 +329,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest
 	}
 
 	// Restore deleted files which are still accessible via open application FDs.
-	dirsToDelete := make(map[*dentry]struct{})
+	dirsToDelete := make(map[*dentry]string)
 	for d := range fs.savedDeletedOpenDentries {
 		if err := d.restoreDeleted(ctx, &opts, dirsToDelete); err != nil {
 			return err
@@ -344,7 +346,10 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest
 			delete(leafDirectories, d.parent.Load())
 		}
 		for leafD := range leafDirectories {
-			if err := leafD.parent.Load().unlink(ctx, leafD.name, linux.AT_REMOVEDIR); err != nil {
+			// Note that we use the name specified in dirsToDelete map, which is the
+			// name used to create the temporary directory. This name may differ from
+			// leafD.name if another non-deleted directory already exists there.
+			if err := leafD.parent.Load().unlink(ctx, dirsToDelete[leafD], linux.AT_REMOVEDIR); err != nil {
 				return fmt.Errorf("failed to clean up recreated deleted directory %q: %v", genericDebugPathname(fs, leafD), err)
 			}
 			delete(dirsToDelete, leafD)
@@ -384,7 +389,7 @@ func (d *dentry) restoreDescendantsRecursive(ctx context.Context, opts *vfs.Comp
 // Preconditions:
 //   - d.isRegularFile() || d.isDir()
 //   - d.savedDeletedData != nil iff d.isRegularFile()
-func (d *dentry) restoreDeleted(ctx context.Context, opts *vfs.CompleteRestoreOptions, dirsToDelete map[*dentry]struct{}) error {
+func (d *dentry) restoreDeleted(ctx context.Context, opts *vfs.CompleteRestoreOptions, dirsToDelete map[*dentry]string) error {
 	parent := d.parent.Load()
 	if _, ok := d.fs.savedDeletedOpenDentries[parent]; ok {
 		// Recursively restore the parent first if the parent is also deleted.
@@ -402,10 +407,26 @@ func (d *dentry) restoreDeleted(ctx context.Context, opts *vfs.CompleteRestoreOp
 	}
 }
 
-func (d *dentry) restoreDeletedDirectory(ctx context.Context, opts *vfs.CompleteRestoreOptions, dirsToDelete map[*dentry]struct{}) error {
+func randomNameForDeleted(name string) string {
+	var randBuf [8]byte
+	rand.Read(randBuf[:])
+	return fmt.Sprintf("%s.tmp-gvisor-restore-%s", name, hex.EncodeToString(randBuf[:]))
+}
+
+func (d *dentry) restoreDeletedDirectory(ctx context.Context, opts *vfs.CompleteRestoreOptions, dirsToDelete map[*dentry]string) error {
 	// Recreate the directory on the host filesystem. This will be deleted later.
 	parent := d.parent.Load()
 	_, err := parent.mkdir(ctx, d.name, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
+	if linuxerr.Equals(linuxerr.EEXIST, err) {
+		// Change d.name for the remainder of this function.
+		origName := d.name
+		d.name = randomNameForDeleted(d.name)
+		defer func() {
+			d.name = origName
+		}()
+		log.Warningf("Deleted directory %q was replaced with a new directory at the same path, using new name %q", genericDebugPathname(d.fs, d), d.name)
+		_, err = parent.mkdir(ctx, d.name, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
+	}
 	if err != nil {
 		return fmt.Errorf("failed to re-create deleted directory %q: %w", genericDebugPathname(d.fs, d), err)
 	}
@@ -418,28 +439,32 @@ func (d *dentry) restoreDeletedDirectory(ctx context.Context, opts *vfs.Complete
 	}
 	// We will delete the directory later. We need to keep it around in case any
 	// of its children need to be restored after this.
-	dirsToDelete[d] = struct{}{}
+	dirsToDelete[d] = d.name
 	delete(d.fs.savedDeletedOpenDentries, d)
 	return nil
 }
 
 func (d *dentry) restoreDeletedRegularFile(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
 	// Recreate the file on the host filesystem (this is temporary).
 	parent := d.parent.Load()
-	name := d.name
-	_, h, err := parent.openCreate(ctx, name, linux.O_WRONLY, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
+	_, h, err := parent.openCreate(ctx, d.name, linux.O_WRONLY, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
 	if linuxerr.Equals(linuxerr.EEXIST, err) {
-		name = fmt.Sprintf("%s.tmp-gvisor-restore", name)
-		log.Warningf("Deleted file %q was replaced with a new file at the same path, using new name %q", genericDebugPathname(d.fs, d), name)
-		_, h, err = parent.openCreate(ctx, name, linux.O_WRONLY, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
+		// Change d.name for the remainder of this function.
+		origName := d.name
+		d.name = randomNameForDeleted(d.name)
+		defer func() {
+			d.name = origName
+		}()
+		log.Warningf("Deleted file %q was replaced with a new file at the same path, using new name %q", genericDebugPathname(d.fs, d), d.name)
+		_, h, err = parent.openCreate(ctx, d.name, linux.O_WRONLY, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load()), false /* createDentry */)
 	}
 	if err != nil {
 		return fmt.Errorf("failed to re-create deleted file %q: %w", genericDebugPathname(d.fs, d), err)
 	}
 	defer h.close(ctx)
 	// In case of errors, clean up the recreated file.
 	unlinkCU := cleanup.Make(func() {
-		if err := parent.unlink(ctx, name, 0 /* flags */); err != nil {
+		if err := parent.unlink(ctx, d.name, 0 /* flags */); err != nil {
 			log.Warningf("failed to clean up recreated deleted file %q: %v", genericDebugPathname(d.fs, d), err)
 		}
 	})
@@ -462,7 +487,7 @@ func (d *dentry) restoreDeletedRegularFile(ctx context.Context, opts *vfs.Comple
 	}
 	// Finally, unlink the recreated file.
 	unlinkCU.Release()
-	if err := parent.unlink(ctx, name, 0 /* flags */); err != nil {
+	if err := parent.unlink(ctx, d.name, 0 /* flags */); err != nil {
 		return fmt.Errorf("failed to clean up recreated deleted file %q: %v", genericDebugPathname(d.fs, d), err)
 	}
 	delete(d.fs.savedDeletedOpenDentries, d)

test/syscalls/linux/unlink.cc

@@ -239,6 +239,62 @@ TEST(UnlinkTest, UnlinkAtEmptyPath) {
               SyscallFailsWithErrno(ENOENT));
 }
 
+// The primary purpose of this test is to verify that save/restore works for
+// open file descriptors to deleted files and directories.
+TEST(UnlinkTest, UnlinkWithOpenFDs) {
+  // TODO: b/400287667 - Enable save/restore for local gofer.
+  DisableSave ds;
+  if (IsRunningOnRunsc()) {
+    ds.reset();
+  }
+
+  // Create some nested directories.
+  TempPath foo = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  TempPath bar = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(foo.path()));
+  TempPath baz = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(bar.path()));
+
+  // Create a file and directory in the inner most directory.
+  const std::string file_path = JoinPath(baz.path(), "file");
+  FileDescriptor file_fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file_path, O_RDWR | O_CREAT, 0666));
+  const char kHello[] = "hello";
+  ASSERT_THAT(write(file_fd.get(), kHello, sizeof(kHello)),
+              SyscallSucceedsWithValue(sizeof(kHello)));
+
+  const std::string dir_path = JoinPath(baz.path(), "dir");
+  ASSERT_THAT(mkdir(dir_path.c_str(), 0777), SyscallSucceeds());
+  FileDescriptor dir_fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(dir_path, O_RDONLY | O_DIRECTORY));
+
+  // Unlink "file" and "dir".
+  EXPECT_THAT(unlink(file_path.c_str()), SyscallSucceeds());
+  EXPECT_THAT(rmdir(dir_path.c_str()), SyscallSucceeds());
+
+  // Recreate files in the same position. S/R should be able to handle this.
+  FileDescriptor new_file_fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file_path, O_RDWR | O_CREAT, 0666));
+  const char kWorld[] = "world";
+  ASSERT_THAT(write(new_file_fd.get(), kWorld, sizeof(kWorld)),
+              SyscallSucceedsWithValue(sizeof(kWorld)));
+  new_file_fd.reset();
+  ASSERT_THAT(mkdir(dir_path.c_str(), 0777), SyscallSucceeds());
+
+  // Unlink "file" and "dir" again.
+  EXPECT_THAT(unlink(file_path.c_str()), SyscallSucceeds());
+  EXPECT_THAT(rmdir(dir_path.c_str()), SyscallSucceeds());
+
+  // Delete the remaining directories.
+  EXPECT_THAT(rmdir(baz.path().c_str()), SyscallSucceeds());
+  EXPECT_THAT(rmdir(bar.path().c_str()), SyscallSucceeds());
+  EXPECT_THAT(rmdir(foo.path().c_str()), SyscallSucceeds());
+
+  // Verify that the original file contents were preserved across unlink/rmdir.
+  char buf[sizeof(kHello)] = {};
+  ASSERT_THAT(pread(file_fd.get(), buf, sizeof(buf), 0),
+              SyscallSucceedsWithValue(sizeof(buf)));
+  EXPECT_STREQ(buf, kHello);
+}
+
 }  // namespace
 
 }  // namespace testing