Fix directfs restore for deleted regular file when read handle is not available

In such a situation, we can not rely on traditional methods (like openHandle())
to open a readable handle. This is because, for directfs to open a handle, it
needs to re-walk the file via the parent using openat(parentFD, name). This
does not work for deleted files, it will fail with ENOENT. runsc gofer works
around this by using /proc/self/fd/ to re-open the control FD in the desired
mode. However, the sentry does not have access to any procfs instance (for
security). See fcbc289 ("runsc: umount /proc in the sandbox namespace").

This change makes directfs just use the control FD to fetch file data. The
control FD should not be used for IO. We make an exception here for S/R.

Fixes #11903

PiperOrigin-RevId: 785037982

Author: ayushr2

pkg/sentry/fsimpl/gofer/dentry_impl.go

@@ -589,6 +589,38 @@ func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptio
 	}
 }
 
+// Precondition: d.handleMu is read locked.
+func (d *dentry) readHandleForDeleted(ctx context.Context) (handle, error) {
+	if d.isReadHandleOk() {
+		return d.readHandle(), nil
+	}
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		// ensureSharedHandle locks handleMu for write. Unlock it temporarily.
+		d.handleMu.RUnlock()
+		err := d.ensureSharedHandle(ctx, true /* read */, false /* write */, false /* trunc */)
+		d.handleMu.RLock()
+		if err != nil {
+			return handle{}, fmt.Errorf("failed to open read handle: %w", err)
+		}
+		return d.readHandle(), nil
+	case *directfsDentry:
+		// The sentry does not have access to any procfs mount which it could use
+		// to re-open dt.controlFD with a different mode (via /proc/self/fd/). The
+		// file is unlinked, so we can't use openat(parent.controlFD, name) either.
+		// dt.controlFD must be a read-only FD (see tryOpen() documentation). Just
+		// seek the control FD to 0 and return it. The control FD is not used for
+		// reading by the sentry, so this should be safe.
+		// TODO(b/431481259): Use dentry.ensureSharedHandle() here as well.
+		if _, err := unix.Seek(dt.controlFD, 0, unix.SEEK_SET); err != nil {
+			return handle{}, fmt.Errorf("failed to seek control FD to 0: %w", err)
+		}
+		return handle{fd: int32(dt.controlFD)}, nil
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
 // doRevalidation calls into r.start's dentry implementation to perform
 // revalidation on all the dentries contained in r.
 //

pkg/sentry/fsimpl/gofer/directfs_dentry.go

@@ -169,6 +169,7 @@ func (d *directfsDentry) openHandle(ctx context.Context, flags uint32) (handle,
 
 	// The only way to re-open an FD with different flags is via procfs or
 	// openat(2) from the parent. Procfs does not exist here. So use parent.
+	// TODO(b/431481259): This does not work for deleted files.
 	flags |= hostOpenFlags
 	openFD, err := unix.Openat(parent.impl.(*directfsDentry).controlFD, d.name, int(flags), 0)
 	if err != nil {

pkg/sentry/fsimpl/gofer/gofer.go

@@ -1971,20 +1971,11 @@ func (d *dentry) removeXattr(ctx context.Context, creds *auth.Credentials, name
 func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool) error {
 	// O_TRUNC unconditionally requires us to obtain a new handle (opened with
 	// O_TRUNC).
-	if !trunc {
-		d.handleMu.RLock()
-		canReuseCurHandle := (!read || d.isReadHandleOk()) && (!write || d.isWriteHandleOk())
-		d.handleMu.RUnlock()
-		if canReuseCurHandle {
-			// Current handles are sufficient.
-			return nil
-		}
-	}
-
 	d.handleMu.Lock()
 	needNewHandle := (read && !d.isReadHandleOk()) || (write && !d.isWriteHandleOk()) || trunc
 	if !needNewHandle {
 		d.handleMu.Unlock()
+		// Current handles are sufficient.
 		return nil
 	}
 

pkg/sentry/fsimpl/gofer/save_restore.go

@@ -152,16 +152,9 @@ 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
-	if d.isReadHandleOk() {
-		h = d.readHandle()
-	} else {
-		var err error
-		h, err = d.openHandle(ctx, true /* read */, false /* write */, false /* trunc */)
-		if err != nil {
-			return fmt.Errorf("failed to open read handle for deleted file %q: %w", genericDebugPathname(d.fs, d), err)
-		}
-		defer h.close(ctx)
+	h, err := d.readHandleForDeleted(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to open read handle for deleted file %q: %w", genericDebugPathname(d.fs, d), err)
 	}
 	// Read the file data and store it in d.savedDeletedData.
 	d.dataMu.RLock()

test/syscalls/linux/unlink.cc

@@ -295,6 +295,40 @@ TEST(UnlinkTest, UnlinkWithOpenFDs) {
   EXPECT_STREQ(buf, kHello);
 }
 
+// The primary goal of this test is to ensure that a write-only FD to a deleted
+// file is savable.
+TEST(UnlinkTest, UnlinkWithOpenFDsWriteOnly) {
+  // TODO(b/400287667): Enable save/restore for local gofer.
+  DisableSave ds;
+  if (IsRunningOnRunsc()) {
+    ds.reset();
+  }
+
+  // Create a file.
+  TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
+
+  // Open the file with O_WRONLY.
+  FileDescriptor file_fd =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_WRONLY, 0222));
+
+  // Unlink the file.
+  EXPECT_THAT(unlink(file.path().c_str()), SyscallSucceeds());
+
+  // Write to the file.
+  const char kHello[] = "hello";
+  ASSERT_THAT(write(file_fd.get(), kHello, sizeof(kHello)),
+              SyscallSucceedsWithValue(sizeof(kHello)));
+
+  // TODO(b/400287667): When running with local gofer AND cache policy = none,
+  // stat-ing a deleted file returns ENOENT.
+  if (IsRunningOnRunsc()) {
+    // Stat the file to verify its size.
+    struct stat st;
+    ASSERT_THAT(fstat(file_fd.get(), &st), SyscallSucceeds());
+    EXPECT_EQ(st.st_size, sizeof(kHello));
+  }
+}
+
 }  // namespace
 
 }  // namespace testing