gofer: Add support for restoring deleted directories.

Fixes #11425

PiperOrigin-RevId: 740865278

Author: ayushr2

pkg/sentry/fsimpl/gofer/dentry_impl.go

@@ -369,12 +369,12 @@ func (d *dentry) link(ctx context.Context, target *dentry, name string) (*dentry
 }
 
 // Precondition: !d.isSynthetic().
-func (d *dentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) {
+func (d *dentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID, createDentry bool) (*dentry, error) {
 	switch dt := d.impl.(type) {
 	case *lisafsDentry:
-		return dt.mkdir(ctx, name, mode, uid, gid)
+		return dt.mkdir(ctx, name, mode, uid, gid, createDentry)
 	case *directfsDentry:
-		return dt.mkdir(name, mode, uid, gid)
+		return dt.mkdir(name, mode, uid, gid, createDentry)
 	default:
 		panic("unknown dentry implementation")
 	}

pkg/sentry/fsimpl/gofer/directfs_dentry.go

@@ -565,11 +565,11 @@ func (d *directfsDentry) link(target *directfsDentry, name string) (*dentry, err
 	return d.getCreatedChild(name, auth.NoID /* uid */, auth.NoID /* gid */, false /* isDir */, true /* createDentry */)
 }
 
-func (d *directfsDentry) mkdir(name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) {
+func (d *directfsDentry) mkdir(name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID, createDentry bool) (*dentry, error) {
 	if err := unix.Mkdirat(d.controlFD, name, uint32(mode)); err != nil {
 		return nil, err
 	}
-	return d.getCreatedChild(name, uid, gid, true /* isDir */, true /* createDentry */)
+	return d.getCreatedChild(name, uid, gid, true /* isDir */, createDentry)
 }
 
 func (d *directfsDentry) symlink(name, target string, creds *auth.Credentials) (*dentry, error) {

pkg/sentry/fsimpl/gofer/filesystem.go

@@ -845,7 +845,7 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
 			mode |= linux.S_ISGID
 		}
 
-		child, err := parent.mkdir(ctx, name, mode, creds.EffectiveKUID, kgid)
+		child, err := parent.mkdir(ctx, name, mode, creds.EffectiveKUID, kgid, true /* createDentry */)
 		if err == nil {
 			if fs.opts.interop != InteropModeShared {
 				parent.incLinks()

pkg/sentry/fsimpl/gofer/gofer.go

@@ -982,9 +982,9 @@ type dentry struct {
 	// tracks dirty segments in cache. dirty is protected by dataMu.
 	dirty fsutil.DirtySet
 
-	// If this dentry represents a deleted regular file, deletedDataSR is used to
-	// store file data for save/restore.
-	deletedDataSR []byte
+	// If this dentry represents a deleted regular file, savedDeletedData is used
+	// to store file data for save/restore.
+	savedDeletedData []byte
 
 	// pf implements memmap.File for mappings of hostFD.
 	pf dentryPlatformFile

pkg/sentry/fsimpl/gofer/lisafs_dentry.go

@@ -432,11 +432,14 @@ func (d *lisafsDentry) link(ctx context.Context, target *lisafsDentry, name stri
 	return d.newChildDentry(ctx, &linkInode, name)
 }
 
-func (d *lisafsDentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) {
+func (d *lisafsDentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID, createDentry bool) (*dentry, error) {
 	childDirInode, err := d.controlFD.MkdirAt(ctx, name, mode, lisafs.UID(uid), lisafs.GID(gid))
 	if err != nil {
 		return nil, err
 	}
+	if !createDentry {
+		return nil, nil
+	}
 	return d.newChildDentry(ctx, &childDirInode, name)
 }
 
@@ -458,13 +461,13 @@ func (d *lisafsDentry) openCreate(ctx context.Context, name string, flags uint32
 		fdLisa: d.fs.client.NewFD(openFD),
 		fd:     int32(hostFD),
 	}
-	var child *dentry
-	if createDentry {
-		child, err = d.fs.newLisafsDentry(ctx, &ino)
-		if err != nil {
-			h.close(ctx)
-			return nil, noHandle, err
-		}
+	if !createDentry {
+		return nil, h, nil
+	}
+	child, err := d.fs.newLisafsDentry(ctx, &ino)
+	if err != nil {
+		h.close(ctx)
+		return nil, noHandle, err
 	}
 	return child, h, nil
 }

pkg/sentry/fsimpl/gofer/save_restore.go

@@ -112,14 +112,21 @@ func (fd *specialFileFD) savePipeData(ctx context.Context) error {
 }
 
 func (d *dentry) prepareSaveDead(ctx context.Context) error {
-	if !d.isRegularFile() {
-		return fmt.Errorf("gofer.dentry(%q).prepareSaveDead: only regular deleted dentries can be saved, got %s", genericDebugPathname(d.fs, d), linux.FileMode(d.mode.Load()))
+	if !d.isRegularFile() && !d.isDir() {
+		return fmt.Errorf("gofer.dentry(%q).prepareSaveDead: only deleted dentries for regular files and directories can be saved, got %s", genericDebugPathname(d.fs, d), linux.FileMode(d.mode.Load()))
 	}
 	if !d.isDeleted() {
 		return fmt.Errorf("gofer.dentry(%q).prepareSaveDead: invalidated dentries can't be saved", genericDebugPathname(d.fs, d))
 	}
-	if !d.cachedMetadataAuthoritative() {
-		if err := d.updateMetadata(ctx); err != nil {
+	if d.isRegularFile() {
+		if !d.cachedMetadataAuthoritative() {
+			// Get updated metadata for d in case we need to perform metadata
+			// validation during restore.
+			if err := d.updateMetadata(ctx); err != nil {
+				return err
+			}
+		}
+		if err := d.prepareSaveDeletedRegularFile(ctx); err != nil {
 			return err
 		}
 	}
@@ -129,6 +136,18 @@ func (d *dentry) prepareSaveDead(ctx context.Context) error {
 			write: d.isWriteHandleOk(),
 		}
 	}
+	if d.fs.savedDeletedOpenDentries == nil {
+		d.fs.savedDeletedOpenDentries = make(map[*dentry]struct{})
+	}
+	d.fs.savedDeletedOpenDentries[d] = struct{}{}
+	return nil
+}
+
+// Preconditions:
+//   - d.isRegularFile()
+//   - d.isDeleted()
+func (d *dentry) prepareSaveDeletedRegularFile(ctx context.Context) error {
+	// Fetch an appropriate handle to read the deleted file.
 	d.handleMu.RLock()
 	defer d.handleMu.RUnlock()
 	var h handle
@@ -142,12 +161,13 @@ func (d *dentry) prepareSaveDead(ctx context.Context) error {
 		}
 		defer h.close(ctx)
 	}
+	// Read the file data and store it in d.savedDeletedData.
 	d.dataMu.RLock()
 	defer d.dataMu.RUnlock()
-	d.deletedDataSR = make([]byte, d.size.Load())
+	d.savedDeletedData = make([]byte, d.size.Load())
 	done := uint64(0)
-	for done < uint64(len(d.deletedDataSR)) {
-		n, err := h.readToBlocksAt(ctx, safemem.BlockSeqOf(safemem.BlockFromSafeSlice(d.deletedDataSR[done:])), done)
+	for done < uint64(len(d.savedDeletedData)) {
+		n, err := h.readToBlocksAt(ctx, safemem.BlockSeqOf(safemem.BlockFromSafeSlice(d.savedDeletedData[done:])), done)
 		done += n
 		if err != nil {
 			if err == io.EOF {
@@ -156,14 +176,9 @@ func (d *dentry) prepareSaveDead(ctx context.Context) error {
 			return fmt.Errorf("failed to read deleted file %q: %w", genericDebugPathname(d.fs, d), err)
 		}
 	}
-	if done < uint64(len(d.deletedDataSR)) {
-		return fmt.Errorf("failed to read all of deleted file %q: read %d bytes, expected %d", genericDebugPathname(d.fs, d), done, len(d.deletedDataSR))
-	}
-	d.deletedDataSR = d.deletedDataSR[:done]
-	if d.fs.savedDeletedOpenDentries == nil {
-		d.fs.savedDeletedOpenDentries = make(map[*dentry]struct{})
+	if done < uint64(len(d.savedDeletedData)) {
+		return fmt.Errorf("failed to read all of deleted file %q: read %d bytes, expected %d", genericDebugPathname(d.fs, d), done, len(d.savedDeletedData))
 	}
-	d.fs.savedDeletedOpenDentries[d] = struct{}{}
 	return nil
 }
 
@@ -200,15 +215,17 @@ func (d *dentry) prepareSaveRecursive(ctx context.Context) error {
 
 // beforeSave is invoked by stateify.
 func (d *dentry) beforeSave() {
-	if d.vfsd.IsDead() && d.deletedDataSR == nil {
-		panic(fmt.Sprintf("gofer.dentry(%q).beforeSave: deletedDataSR is nil for dead dentry (deleted=%t, synthetic=%t)", genericDebugPathname(d.fs, d), d.isDeleted(), d.isSynthetic()))
+	if d.vfsd.IsDead() {
+		if _, ok := d.fs.savedDeletedOpenDentries[d]; !ok {
+			panic(fmt.Sprintf("gofer.dentry(%q).beforeSave: dead dentry is not saved in fs.savedDeletedOpenDentries (deleted=%t, synthetic=%t)", genericDebugPathname(d.fs, d), d.isDeleted(), d.isSynthetic()))
+		}
 	}
 }
 
 // BeforeResume implements vfs.FilesystemImplSaveRestoreExtension.BeforeResume.
 func (fs *filesystem) BeforeResume(ctx context.Context) {
 	for d := range fs.savedDeletedOpenDentries {
-		d.deletedDataSR = nil
+		d.savedDeletedData = nil
 	}
 	fs.savedDeletedOpenDentries = nil
 	fs.savedDentryRW = nil
@@ -310,11 +327,29 @@ 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{})
 	for d := range fs.savedDeletedOpenDentries {
-		if err := d.restoreDead(ctx, &opts); err != nil {
+		if err := d.restoreDeleted(ctx, &opts, dirsToDelete); err != nil {
 			return err
 		}
 	}
+	for len(dirsToDelete) > 0 {
+		// In case of nested deleted directories, only leaf directories can be
+		// deleted. Then repeat as parent directories become leaves.
+		leafDirectories := make(map[*dentry]struct{})
+		for d := range dirsToDelete {
+			leafDirectories[d] = struct{}{}
+		}
+		for d := range dirsToDelete {
+			delete(leafDirectories, d.parent.Load())
+		}
+		for leafD := range leafDirectories {
+			if err := leafD.parent.Load().unlink(ctx, leafD.name, linux.AT_REMOVEDIR); err != nil {
+				return fmt.Errorf("failed to clean up recreated deleted directory %q: %v", genericDebugPathname(fs, leafD), err)
+			}
+			delete(dirsToDelete, leafD)
+		}
+	}
 
 	// Discard state only required during restore.
 	fs.savedDeletedOpenDentries = nil
@@ -344,10 +379,51 @@ func (d *dentry) restoreDescendantsRecursive(ctx context.Context, opts *vfs.Comp
 	return nil
 }
 
-// restoreDead restores a deleted regular file.
+// restoreDeleted restores a deleted dentry for a directory or regular file.
 //
-// Preconditions: d.deletedDataSR != nil.
-func (d *dentry) restoreDead(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
+// 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 {
+	parent := d.parent.Load()
+	if _, ok := d.fs.savedDeletedOpenDentries[parent]; ok {
+		// Recursively restore the parent first if the parent is also deleted.
+		if err := parent.restoreDeleted(ctx, opts, dirsToDelete); err != nil {
+			return err
+		}
+	}
+	switch {
+	case d.isRegularFile():
+		return d.restoreDeletedRegularFile(ctx, opts)
+	case d.isDir():
+		return d.restoreDeletedDirectory(ctx, opts, dirsToDelete)
+	default:
+		return fmt.Errorf("gofer.dentry(%q).restoreDeleted: invalid file type %s", genericDebugPathname(d.fs, d), linux.FileMode(d.mode.Load()))
+	}
+}
+
+func (d *dentry) restoreDeletedDirectory(ctx context.Context, opts *vfs.CompleteRestoreOptions, dirsToDelete map[*dentry]struct{}) 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 err != nil {
+		return fmt.Errorf("failed to re-create deleted directory %q: %w", genericDebugPathname(d.fs, d), err)
+	}
+	// Restore the directory.
+	if err := d.restoreFile(ctx, opts); err != nil {
+		if err := parent.unlink(ctx, d.name, linux.AT_REMOVEDIR); err != nil {
+			log.Warningf("failed to clean up recreated deleted directory %q: %v", genericDebugPathname(d.fs, d), err)
+		}
+		return fmt.Errorf("failed to restore deleted directory: %w", err)
+	}
+	// 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{}{}
+	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()
 	_, 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 */)
@@ -363,26 +439,27 @@ func (d *dentry) restoreDead(ctx context.Context, opts *vfs.CompleteRestoreOptio
 	})
 	defer unlinkCU.Clean()
 	// Write the file data to the recreated file.
-	n, err := h.writeFromBlocksAt(ctx, safemem.BlockSeqOf(safemem.BlockFromSafeSlice(d.deletedDataSR)), 0)
+	n, err := h.writeFromBlocksAt(ctx, safemem.BlockSeqOf(safemem.BlockFromSafeSlice(d.savedDeletedData)), 0)
 	if err != nil {
 		return fmt.Errorf("failed to write deleted file %q: %w", genericDebugPathname(d.fs, d), err)
 	}
-	if n != uint64(len(d.deletedDataSR)) {
-		return fmt.Errorf("failed to write all of deleted file %q: wrote %d bytes, expected %d", genericDebugPathname(d.fs, d), n, len(d.deletedDataSR))
+	if n != uint64(len(d.savedDeletedData)) {
+		return fmt.Errorf("failed to write all of deleted file %q: wrote %d bytes, expected %d", genericDebugPathname(d.fs, d), n, len(d.savedDeletedData))
 	}
-	d.deletedDataSR = nil
+	d.savedDeletedData = nil
 	// Restore the file. Note that timestamps may not match since we re-created
 	// the file on the host.
 	recreateOpts := *opts
 	recreateOpts.ValidateFileModificationTimestamps = false
 	if err := d.restoreFile(ctx, &recreateOpts); err != nil {
-		return err
+		return fmt.Errorf("failed to restore deleted regular file: %w", err)
 	}
 	// Finally, unlink the recreated file.
 	unlinkCU.Release()
 	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)
 	return nil
 }
 

test/syscalls/linux/unlink.cc

@@ -168,13 +168,27 @@ TEST(UnlinkTest, OpenFile) {
   if (IsRunningOnRunsc()) {
     ds.reset();
   }
-  auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
+  TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
   int fd;
   EXPECT_THAT(fd = open(file.path().c_str(), O_RDWR, 0666), SyscallSucceeds());
   EXPECT_THAT(unlink(file.path().c_str()), SyscallSucceeds());
   EXPECT_THAT(close(fd), SyscallSucceeds());
 }
 
+TEST(RmdirTest, OpenDirectory) {
+  // TODO(b/400287667): Enable save/restore for local gofer.
+  DisableSave ds;
+  if (IsRunningOnRunsc()) {
+    ds.reset();
+  }
+  TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  int fd;
+  EXPECT_THAT(fd = open(dir.path().c_str(), O_RDONLY | O_DIRECTORY),
+              SyscallSucceeds());
+  EXPECT_THAT(rmdir(dir.path().c_str()), SyscallSucceeds());
+  EXPECT_THAT(close(fd), SyscallSucceeds());
+}
+
 TEST(UnlinkTest, CannotRemoveDots) {
   auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
   const std::string self = JoinPath(file.path(), ".");