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

13 files changed, 144 insertions, 74 deletions

diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go
index d6a19dc81..15a0129ce 100644
--- a/pkg/sentry/fs/dirent.go
+++ b/pkg/sentry/fs/dirent.go
@@ -1563,6 +1563,7 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string
 		}
 
 		// newName doesn't exist; simply create it below.
+		replaced = nil
 	} else {
 		// Check constraints on the dirent being replaced.
 
@@ -1620,7 +1621,7 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string
 		replaced.DecRef()
 	}
 
-	if err := renamed.Inode.Rename(ctx, oldParent, renamed, newParent, newName); err != nil {
+	if err := renamed.Inode.Rename(ctx, oldParent, renamed, newParent, newName, replaced != nil); err != nil {
 		return err
 	}
 
diff --git a/pkg/sentry/fs/fsutil/inode.go b/pkg/sentry/fs/fsutil/inode.go
index bd3bd1bb2..c1ad45e52 100644
--- a/pkg/sentry/fs/fsutil/inode.go
+++ b/pkg/sentry/fs/fsutil/inode.go
@@ -338,7 +338,7 @@ func (InodeNotDirectory) RemoveDirectory(context.Context, *fs.Inode, string) err
 }
 
 // Rename implements fs.FileOperations.Rename.
-func (InodeNotDirectory) Rename(context.Context, *fs.Inode, string, *fs.Inode, string) error {
+func (InodeNotDirectory) Rename(context.Context, *fs.Inode, string, *fs.Inode, string, bool) error {
 	return syserror.EINVAL
 }
 
@@ -378,7 +378,7 @@ func (InodeNoopTruncate) Truncate(context.Context, *fs.Inode, int64) error {
 type InodeNotRenameable struct{}
 
 // Rename implements fs.InodeOperations.Rename.
-func (InodeNotRenameable) Rename(context.Context, *fs.Inode, string, *fs.Inode, string) error {
+func (InodeNotRenameable) Rename(context.Context, *fs.Inode, string, *fs.Inode, string, bool) error {
 	return syserror.EINVAL
 }
 
diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go
index 43f990d16..2ba400836 100644
--- a/pkg/sentry/fs/gofer/path.go
+++ b/pkg/sentry/fs/gofer/path.go
@@ -298,7 +298,7 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
 }
 
 // Rename renames this node.
-func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
+func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
 	// Unwrap the new parent to a *inodeOperations.
 	newParentInodeOperations, ok := newParent.InodeOperations.(*inodeOperations)
 	if !ok {
@@ -323,7 +323,12 @@ func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldNa
 			oldParentInodeOperations.cachingInodeOps.DecLinks(ctx)
 		}
 		if i.session().cachePolicy.cacheUAttrs(newParent) {
-			newParentInodeOperations.cachingInodeOps.IncLinks(ctx)
+			// Only IncLinks if there is a new addition to
+			// newParent. If this is replacement, then the total
+			// count remains the same.
+			if !replacement {
+				newParentInodeOperations.cachingInodeOps.IncLinks(ctx)
+			}
 		}
 	}
 	if i.session().cachePolicy.cacheReaddir() {
diff --git a/pkg/sentry/fs/host/inode.go b/pkg/sentry/fs/host/inode.go
index 6ff6c3254..2030edcb4 100644
--- a/pkg/sentry/fs/host/inode.go
+++ b/pkg/sentry/fs/host/inode.go
@@ -296,7 +296,7 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
+func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
 	op, ok := oldParent.InodeOperations.(*inodeOperations)
 	if !ok {
 		return syscall.EXDEV
diff --git a/pkg/sentry/fs/inode.go b/pkg/sentry/fs/inode.go
index 08b5c5902..b8b5c1528 100644
--- a/pkg/sentry/fs/inode.go
+++ b/pkg/sentry/fs/inode.go
@@ -152,7 +152,8 @@ func (i *Inode) WriteOut(ctx context.Context) error {
 // Lookup calls i.InodeOperations.Lookup with i as the directory.
 func (i *Inode) Lookup(ctx context.Context, name string) (*Dirent, error) {
 	if i.overlay != nil {
-		return overlayLookup(ctx, i.overlay, i, name)
+		d, _, err := overlayLookup(ctx, i.overlay, i, name)
+		return d, err
 	}
 	return i.InodeOperations.Lookup(ctx, i, name)
 }
@@ -211,11 +212,11 @@ func (i *Inode) Remove(ctx context.Context, d *Dirent, remove *Dirent) error {
 }
 
 // Rename calls i.InodeOperations.Rename with the given arguments.
-func (i *Inode) Rename(ctx context.Context, oldParent *Dirent, renamed *Dirent, newParent *Dirent, newName string) error {
+func (i *Inode) Rename(ctx context.Context, oldParent *Dirent, renamed *Dirent, newParent *Dirent, newName string, replacement bool) error {
 	if i.overlay != nil {
-		return overlayRename(ctx, i.overlay, oldParent, renamed, newParent, newName)
+		return overlayRename(ctx, i.overlay, oldParent, renamed, newParent, newName, replacement)
 	}
-	return i.InodeOperations.Rename(ctx, oldParent.Inode, renamed.name, newParent.Inode, newName)
+	return i.InodeOperations.Rename(ctx, oldParent.Inode, renamed.name, newParent.Inode, newName, replacement)
 }
 
 // Bind calls i.InodeOperations.Bind with i as the directory.
diff --git a/pkg/sentry/fs/inode_operations.go b/pkg/sentry/fs/inode_operations.go
index db40b5256..548f1eb8b 100644
--- a/pkg/sentry/fs/inode_operations.go
+++ b/pkg/sentry/fs/inode_operations.go
@@ -133,12 +133,15 @@ type InodeOperations interface {
 	// removed is empty.
 	RemoveDirectory(ctx context.Context, dir *Inode, name string) error
 
-	// Rename atomically renames oldName under oldParent to newName
-	// under newParent where oldParent and newParent are directories.
+	// Rename atomically renames oldName under oldParent to newName under
+	// newParent where oldParent and newParent are directories.
 	//
-	// Implementations are responsible for rejecting renames that
-	// replace non-empty directories.
-	Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string) error
+	// If replacement is true, then newName already exists and this call
+	// will replace it with oldName.
+	//
+	// Implementations are responsible for rejecting renames that replace
+	// non-empty directories.
+	Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error
 
 	// Bind binds a new socket under dir at the given name.
 	// Implementations must ensure that name does not already exist.
diff --git a/pkg/sentry/fs/inode_overlay.go b/pkg/sentry/fs/inode_overlay.go
index 92a77917a..6e1dfecf9 100644
--- a/pkg/sentry/fs/inode_overlay.go
+++ b/pkg/sentry/fs/inode_overlay.go
@@ -44,7 +44,11 @@ func overlayWriteOut(ctx context.Context, o *overlayEntry) error {
 	return err
 }
 
-func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name string) (*Dirent, error) {
+// overlayLookup performs a lookup in parent.
+//
+// If name exists, it returns true if the Dirent is in the upper, false if the
+// Dirent is in the lower.
+func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name string) (*Dirent, bool, error) {
 	// Hot path. Avoid defers.
 	parent.copyMu.RLock()
 
@@ -71,7 +75,7 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name
 			// We encountered an error that an overlay cannot handle,
 			// we must propagate it to the caller.
 			parent.copyMu.RUnlock()
-			return nil, err
+			return nil, false, err
 		}
 		if child != nil {
 			if child.IsNegative() {
@@ -93,23 +97,23 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name
 					// that negative Dirent being cached in
 					// the Dirent tree, so we can return
 					// one from the overlay.
-					return NewNegativeDirent(name), nil
+					return NewNegativeDirent(name), false, nil
 				}
 				// Upper fs is not OK with a negative Dirent
 				// being cached in the Dirent tree, so don't
 				// return one.
-				return nil, syserror.ENOENT
+				return nil, false, syserror.ENOENT
 			}
 			entry, err := newOverlayEntry(ctx, upperInode, nil, false)
 			if err != nil {
 				// Don't leak resources.
 				upperInode.DecRef()
 				parent.copyMu.RUnlock()
-				return nil, err
+				return nil, false, err
 			}
 			d, err := NewDirent(newOverlayInode(ctx, entry, inode.MountSource), name), nil
 			parent.copyMu.RUnlock()
-			return d, err
+			return d, true, err
 		}
 	}
 
@@ -127,7 +131,7 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name
 				upperInode.DecRef()
 			}
 			parent.copyMu.RUnlock()
-			return nil, err
+			return nil, false, err
 		}
 		if child != nil {
 			if !child.IsNegative() {
@@ -158,9 +162,9 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name
 		// one as well. See comments above regarding negativeUpperChild
 		// for more info.
 		if negativeUpperChild {
-			return NewNegativeDirent(name), nil
+			return NewNegativeDirent(name), false, nil
 		}
-		return nil, syserror.ENOENT
+		return nil, false, syserror.ENOENT
 	}
 
 	// Did we find a lower Inode? Remember this because we may decide we don't
@@ -195,11 +199,11 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name
 			lowerInode.DecRef()
 		}
 		parent.copyMu.RUnlock()
-		return nil, err
+		return nil, false, err
 	}
 	d, err := NewDirent(newOverlayInode(ctx, entry, inode.MountSource), name), nil
 	parent.copyMu.RUnlock()
-	return d, err
+	return d, upperInode != nil, err
 }
 
 func overlayCreate(ctx context.Context, o *overlayEntry, parent *Dirent, name string, flags FileFlags, perm FilePermissions) (*File, error) {
@@ -317,7 +321,7 @@ func overlayRemove(ctx context.Context, o *overlayEntry, parent *Dirent, child *
 	return nil
 }
 
-func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, renamed *Dirent, newParent *Dirent, newName string) error {
+func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, renamed *Dirent, newParent *Dirent, newName string, replacement bool) error {
 	// To be able to copy these up below, they have to be part of an
 	// overlay file system.
 	//
@@ -327,36 +331,57 @@ func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, rena
 		return syserror.EXDEV
 	}
 
-	// Check here if the file to be replaced exists and is a non-empty
-	// directory. If we copy up first, we may end up copying the directory
-	// but none of its children, so the directory will appear empty in the
-	// upper fs, which will then allow the rename to proceed when it should
-	// return ENOTEMPTY.
-	replaced, err := newParent.Inode.Lookup(ctx, newName)
-	if err != nil && err != syserror.ENOENT {
-		return err
-	}
-	if err == nil {
-		// NOTE: We must drop the reference on replaced before we make
-		// the rename call. For that reason we can't use defer.
-		if !replaced.IsNegative() && IsDir(replaced.Inode.StableAttr) {
-			children, err := readdirOne(ctx, replaced)
-			if err != nil {
-				replaced.DecRef()
-				return err
+	if replacement {
+		// Check here if the file to be replaced exists and is a
+		// non-empty directory. If we copy up first, we may end up
+		// copying the directory but none of its children, so the
+		// directory will appear empty in the upper fs, which will then
+		// allow the rename to proceed when it should return ENOTEMPTY.
+		//
+		// NOTE: Ideally, we'd just pass in the replaced
+		// Dirent from Rename, but we must drop the reference on
+		// replaced before we make the rename call, so Rename can't
+		// pass the Dirent to the Inode without significantly
+		// complicating the API. Thus we look it up again here.
+		//
+		// For the same reason we can't use defer here.
+		replaced, inUpper, err := overlayLookup(ctx, newParent.Inode.overlay, newParent.Inode, newName)
+		// If err == ENOENT or a negative Dirent is returned, then
+		// newName has been removed out from under us. That's fine;
+		// filesystems where that can happen must handle stale
+		// 'replaced'.
+		if err != nil && err != syserror.ENOENT {
+			return err
+		}
+		if err == nil {
+			if !inUpper {
+				// newName doesn't exist in
+				// newParent.Inode.overlay.upper, thus from
+				// that Inode's perspective this won't be a
+				// replacing rename.
+				replacement = false
 			}
 
-			// readdirOne ensures that "." and ".." are not
-			// included among the returned children, so we don't
-			// need to bother checking for them.
-			if len(children) > 0 {
-				replaced.DecRef()
-				return syserror.ENOTEMPTY
+			if !replaced.IsNegative() && IsDir(replaced.Inode.StableAttr) {
+				children, err := readdirOne(ctx, replaced)
+				if err != nil {
+					replaced.DecRef()
+					return err
+				}
+
+				// readdirOne ensures that "." and ".." are not
+				// included among the returned children, so we don't
+				// need to bother checking for them.
+				if len(children) > 0 {
+					replaced.DecRef()
+					return syserror.ENOTEMPTY
+				}
 			}
-		}
 
-		replaced.DecRef()
+			replaced.DecRef()
+		}
 	}
+
 	if err := copyUpLockedForRename(ctx, renamed); err != nil {
 		return err
 	}
@@ -364,7 +389,7 @@ func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, rena
 		return err
 	}
 	oldName := renamed.name
-	if err := o.upper.InodeOperations.Rename(ctx, oldParent.Inode.overlay.upper, oldName, newParent.Inode.overlay.upper, newName); err != nil {
+	if err := o.upper.InodeOperations.Rename(ctx, oldParent.Inode.overlay.upper, oldName, newParent.Inode.overlay.upper, newName, replacement); err != nil {
 		return err
 	}
 	if renamed.Inode.overlay.lowerExists {
diff --git a/pkg/sentry/fs/mock.go b/pkg/sentry/fs/mock.go
index abfdc6a25..118e30f63 100644
--- a/pkg/sentry/fs/mock.go
+++ b/pkg/sentry/fs/mock.go
@@ -132,7 +132,7 @@ func (n *MockInodeOperations) CreateDirectory(context.Context, *Inode, string, F
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (n *MockInodeOperations) Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string) error {
+func (n *MockInodeOperations) Rename(ctx context.Context, oldParent *Inode, oldName string, newParent *Inode, newName string, replacement bool) error {
 	n.renameCalled = true
 	return nil
 }
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.
diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go
index a98fbf0f1..3c84b2977 100644
--- a/pkg/sentry/fs/tmpfs/inode_file.go
+++ b/pkg/sentry/fs/tmpfs/inode_file.go
@@ -107,8 +107,8 @@ func (f *fileInodeOperations) Mappable(*fs.Inode) memmap.Mappable {
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (*fileInodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return rename(ctx, oldParent, oldName, newParent, newName)
+func (*fileInodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return rename(ctx, oldParent, oldName, newParent, newName, replacement)
 }
 
 // GetFile implements fs.InodeOperations.GetFile.
diff --git a/pkg/sentry/fs/tmpfs/tmpfs.go b/pkg/sentry/fs/tmpfs/tmpfs.go
index 1a9d12c0b..a1672a4d0 100644
--- a/pkg/sentry/fs/tmpfs/tmpfs.go
+++ b/pkg/sentry/fs/tmpfs/tmpfs.go
@@ -38,7 +38,7 @@ var fsInfo = fs.Info{
 }
 
 // rename implements fs.InodeOperations.Rename for tmpfs nodes.
-func rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
+func rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
 	op, ok := oldParent.InodeOperations.(*Dir)
 	if !ok {
 		return syserror.EXDEV
@@ -47,7 +47,7 @@ func rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent
 	if !ok {
 		return syserror.EXDEV
 	}
-	return ramfs.Rename(ctx, op.ramfsDir, oldName, np.ramfsDir, newName)
+	return ramfs.Rename(ctx, op.ramfsDir, oldName, np.ramfsDir, newName, replacement)
 }
 
 // Dir is a directory.
@@ -238,8 +238,8 @@ func (d *Dir) newCreateOps() *ramfs.CreateOps {
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (d *Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return rename(ctx, oldParent, oldName, newParent, newName)
+func (d *Dir) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return rename(ctx, oldParent, oldName, newParent, newName, replacement)
 }
 
 // StatFS implments fs.InodeOperations.StatFS.
@@ -266,8 +266,8 @@ func NewSymlink(ctx context.Context, target string, owner fs.FileOwner, msrc *fs
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (s *Symlink) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return rename(ctx, oldParent, oldName, newParent, newName)
+func (s *Symlink) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return rename(ctx, oldParent, oldName, newParent, newName, replacement)
 }
 
 // StatFS returns the tmpfs info.
@@ -295,8 +295,8 @@ func NewSocket(ctx context.Context, socket transport.BoundEndpoint, owner fs.Fil
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (s *Socket) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return rename(ctx, oldParent, oldName, newParent, newName)
+func (s *Socket) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return rename(ctx, oldParent, oldName, newParent, newName, replacement)
 }
 
 // StatFS returns the tmpfs info.
@@ -332,8 +332,8 @@ func NewFifo(ctx context.Context, owner fs.FileOwner, perms fs.FilePermissions,
 }
 
 // Rename implements fs.InodeOperations.Rename.
-func (f *Fifo) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string) error {
-	return rename(ctx, oldParent, oldName, newParent, newName)
+func (f *Fifo) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+	return rename(ctx, oldParent, oldName, newParent, newName, replacement)
 }
 
 // StatFS returns the tmpfs info.
diff --git a/test/BUILD.opensource b/test/BUILD
index 8d2969204..6b83757f6 100644
--- a/test/BUILD.opensource
+++ b/test/BUILD
@@ -1,6 +1,6 @@
 # gVisor is a general-purpose sandbox.
 
-licenses(["notice"])
+package(licenses = ["notice"])
 
 exports_files(["LICENSE"])
 
diff --git a/test/syscalls/linux/rename.cc b/test/syscalls/linux/rename.cc
index f4c877a00..c0cbc7cd9 100644
--- a/test/syscalls/linux/rename.cc
+++ b/test/syscalls/linux/rename.cc
@@ -155,10 +155,11 @@ TEST(RenameTest, DirectoryToOwnChildDirectory) {
 }
 
 TEST(RenameTest, FileOverwritesFile) {
+  auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
   auto f1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
-      GetAbsoluteTestTmpdir(), "first", TempPath::kDefaultFileMode));
+      dir.path(), "first", TempPath::kDefaultFileMode));
   auto f2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
-      GetAbsoluteTestTmpdir(), "second", TempPath::kDefaultFileMode));
+      dir.path(), "second", TempPath::kDefaultFileMode));
   ASSERT_THAT(rename(f1.path().c_str(), f2.path().c_str()), SyscallSucceeds());
   EXPECT_THAT(Exists(f1.path()), IsPosixErrorOkAndHolds(false));
 
@@ -168,6 +169,26 @@ TEST(RenameTest, FileOverwritesFile) {
   EXPECT_EQ("first", f2_contents);
 }
 
+TEST(RenameTest, DirectoryOverwritesDirectoryLinkCount) {
+  auto parent1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(2));
+
+  auto parent2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(2));
+
+  auto dir1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(parent1.path()));
+  auto dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(parent2.path()));
+
+  EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(3));
+  EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(3));
+
+  ASSERT_THAT(rename(dir1.path().c_str(), dir2.path().c_str()),
+              SyscallSucceeds());
+
+  EXPECT_THAT(Links(parent1.path()), IsPosixErrorOkAndHolds(2));
+  EXPECT_THAT(Links(parent2.path()), IsPosixErrorOkAndHolds(3));
+}
+
 TEST(RenameTest, FileDoesNotExist) {
   auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
   const std::string source = JoinPath(dir.path(), "source");