Remove references to replaced child in Rename in ramfs/agentfs

In the case of a rename replacing an existing destination inode, ramfs
Rename failed to first remove the replaced inode. This caused:

1. A leak of a reference to the inode (making it live indefinitely).
2. For directories, a leak of the replaced directory's .. link to the
   parent. This would cause the parent's link count to incorrectly
   increase.

(2) is much simpler to test than (1), so that's what I've done.

agentfs has a similar bug with link count only, so the Dirent layer
informs the Inode if this is a replacing rename.

Fixes #133

PiperOrigin-RevId: 239105698
Change-Id: I4450af2462d8ae3339def812287213d2cbeebde0

1 files changed, 19 insertions, 5 deletions

diff --git a/pkg/sentry/fs/ramfs/dir.go b/pkg/sentry/fs/ramfs/dir.go
index 4da876ebd..b60dab243 100644
--- a/pkg/sentry/fs/ramfs/dir.go
+++ b/pkg/sentry/fs/ramfs/dir.go
@@ -15,6 +15,7 @@
 package ramfs
 
 import (
+	"fmt"
 	"sync"
 	"syscall"
 
@@ -383,8 +384,8 @@ func (d *Dir) GetFile(ctx context.Context, dirent *fs.Dirent, flags fs.FileFlags
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (*Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return Rename(ctx, oldParent.InodeOperations, oldName, newParent.InodeOperations, newName)
+func (*Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return Rename(ctx, oldParent.InodeOperations, oldName, newParent.InodeOperations, newName, replacement)
 }
 
 // dirFileOperations implements fs.FileOperations for a ramfs directory.
@@ -456,7 +457,7 @@ func hasChildren(ctx context.Context, inode *fs.Inode) (bool, error) {
 }
 
 // Rename renames from a *ramfs.Dir to another *ramfs.Dir.
-func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, newParent fs.InodeOperations, newName string) error {
+func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, newParent fs.InodeOperations, newName string, replacement bool) error {
 	op, ok := oldParent.(*Dir)
 	if !ok {
 		return syserror.EXDEV
@@ -469,8 +470,14 @@ func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, n
 	np.mu.Lock()
 	defer np.mu.Unlock()
 
-	// Check whether the ramfs entry to be replaced is a non-empty directory.
-	if replaced, ok := np.children[newName]; ok {
+	// Is this is an overwriting rename?
+	if replacement {
+		replaced, ok := np.children[newName]
+		if !ok {
+			panic(fmt.Sprintf("Dirent claims rename is replacement, but %q is missing from %+v", newName, np))
+		}
+
+		// Non-empty directories cannot be replaced.
 		if fs.IsDir(replaced.StableAttr) {
 			if ok, err := hasChildren(ctx, replaced); err != nil {
 				return err
@@ -478,6 +485,13 @@ func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, n
 				return syserror.ENOTEMPTY
 			}
 		}
+
+		// Remove the replaced child and drop our reference on it.
+		inode, err := np.removeChildLocked(ctx, newName)
+		if err != nil {
+			return err
+		}
+		inode.DecRef()
 	}
 
 	// Be careful, we may have already grabbed this mutex above.