Don't release d.mu in checks for child-existence.

Dirent.exists() is called in Create to check whether a child with the given
name already exists.

Dirent.exists() calls walk(), and before this CL allowed walk() to drop d.mu
while calling d.Inode.Lookup. During this existence check, a racing Rename()
can acquire d.mu and create a new child of the dirent with the same name.
(Note that the source and destination of the rename must be in the same
directory, otherwise renameMu will be taken preventing the race.) In this
case, d.exists() can return false, even though a child with the same name
actually does exist.

This CL changes d.exists() so that it does not release d.mu while walking, thus
preventing the race with Rename.

It also adds comments noting that lockForRename may not take renameMu if the
source and destination are in the same directory, as this is a bit surprising
(at least it was to me).

PiperOrigin-RevId: 241842579
Change-Id: I56524870e39dfcd18cab82054eb3088846c34813

1 files changed, 6 insertions, 1 deletions

diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go
index 15a0129ce..3a1aa6c1e 100644
--- a/pkg/sentry/fs/dirent.go
+++ b/pkg/sentry/fs/dirent.go
@@ -629,7 +629,7 @@ func (d *Dirent) Walk(ctx context.Context, root *Dirent, name string) (*Dirent,
 // - d.mu must be held.
 // - name must must not contain "/"s.
 func (d *Dirent) exists(ctx context.Context, root *Dirent, name string) bool {
-	child, err := d.walk(ctx, root, name, true /* may unlock */)
+	child, err := d.walk(ctx, root, name, false /* may unlock */)
 	if err != nil {
 		// Child may not exist.
 		return false
@@ -1377,8 +1377,13 @@ func (d *Dirent) dropExtendedReference() {
 // lockForRename takes locks on oldParent and newParent as required by Rename
 // and returns a function that will unlock the locks taken. The returned
 // function must be called even if a non-nil error is returned.
+//
+// Note that lockForRename does not take renameMu if the source and destination
+// of the rename are within the same directory.
 func lockForRename(oldParent *Dirent, oldName string, newParent *Dirent, newName string) (func(), error) {
 	if oldParent == newParent {
+		// Rename source and destination are in the same directory. In
+		// this case, we only need to take a lock on that directory.
 		oldParent.mu.Lock()
 		return oldParent.mu.Unlock, nil
 	}