gofer: fix ref drop when racily-unlinked synthetic file is invalidated

PiperOrigin-RevId: 732340885

Author: nixprime

pkg/sentry/fsimpl/gofer/filesystem.go

@@ -714,6 +714,12 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b
 	if child != nil {
 		toDecRef = vfsObj.CommitDeleteDentry(ctx, &child.vfsd) // +checklocksforce: see above.
 		child.setDeleted()
+		// If an extra reference is held on child as described by the comment
+		// for dentry.refs, drop that reference now. We can't race with another
+		// fs.unlinkAt() or invalidation since parent.opMu has been locked for
+		// writing since before we obtained child, and we can't race with
+		// fs.RenameAt() since fs.renameMu has been locked since before we
+		// obtained child.
 		if child.isSynthetic() {
 			parent.syntheticChildren--
 			child.decRefNoCaching()
@@ -1507,6 +1513,10 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
 	toDecRef = vfsObj.CommitRenameReplaceDentry(ctx, &renamed.vfsd, replacedVFSD)
 	if replaced != nil {
 		replaced.setDeleted()
+		// If an extra reference is held on replaced as described by the
+		// comment for dentry.refs, drop that reference now. We can't race with
+		// fs.unlinkAt() or invalidation since fs.renameMu has been locked for
+		// writing since before we obtained replaced.
 		if replaced.isSynthetic() {
 			newParent.syntheticChildren--
 			replaced.decRefNoCaching()

pkg/sentry/fsimpl/gofer/revalidate.go

@@ -198,29 +198,30 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *d
 // Precondition: fs.renameMu must be locked.
 func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem, ds **[]*dentry) {
 	// Remove d from its parent.
-	func() {
+	removed := func() bool {
 		parent := d.parent.Load()
 		parent.opMu.RLock()
 		defer parent.opMu.RUnlock()
 		parent.childrenMu.Lock()
 		defer parent.childrenMu.Unlock()
 
-		if d.isSynthetic() {
-			// Normally we don't mark invalidated dentries as deleted since
-			// they may still exist (but at a different path), and also for
-			// consistency with Linux. However, synthetic files are guaranteed
-			// to become unreachable if their dentries are invalidated, so
-			// treat their invalidation as deletion.
-			d.deleteSynthetic(parent, ds)
-		}
-
 		// Since the opMu was just reacquired above, re-check that the
 		// parent's child with this name is still the same. Do not touch it if
 		// it has been replaced with a different one.
 		if child := parent.children[d.name]; child == d {
 			// Invalidate dentry so it gets reloaded next time it's accessed.
 			delete(parent.children, d.name)
+			if d.isSynthetic() {
+				// Normally we don't mark invalidated dentries as deleted since
+				// they may still exist (but at a different path), and also for
+				// consistency with Linux. However, synthetic files are
+				// guaranteed to become unreachable if their dentries are
+				// invalidated, so treat their invalidation as deletion.
+				d.deleteSynthetic(parent, ds)
+			}
+			return true
 		}
+		return false
 	}()
 
 	// Invalidate d and its descendants.
@@ -239,7 +240,14 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
 			rc.DecRef(ctx)
 		}
 		d.decRefNoCaching()
-		if d.isSynthetic() || d.endpoint != nil {
+
+		// If an extra reference is held on d as described by the comment for
+		// dentry.refs, and d hasn't been racily removed by
+		// filesystem.unlinkAt() or another revalidation, drop that reference
+		// now. (The same would apply to racy replacement by
+		// filesystem.RenameAt(), but we can't race with rename since renameMu
+		// has been locked since entering filesystem.revalidatePath().)
+		if removed && (d.isSynthetic() || d.endpoint != nil) {
 			d.decRefNoCaching()
 		}