From dde836a91858ceee25dbe023263752b39ae21274 Mon Sep 17 00:00:00 2001
From: Adin Scannell <ascannell@google.com>
Date: Mon, 13 Aug 2018 13:29:54 -0700
Subject: Prevent renames across walk fast path.

PiperOrigin-RevId: 208533436
Change-Id: Ifc1a4e2d6438a424650bee831c301b1ac0d670a3
---
 pkg/sentry/fs/dirent.go | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

(limited to 'pkg/sentry/fs')

diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go
index f81ad5792..4d3aeaf41 100644
--- a/pkg/sentry/fs/dirent.go
+++ b/pkg/sentry/fs/dirent.go
@@ -533,14 +533,18 @@ func (d *Dirent) walk(ctx context.Context, root *Dirent, name string, walkMayUnl
 		return nil, syscall.ENOENT
 	}
 
-	// Slow path: load the InodeOperations into memory. Since this is a hot path and the lookup may be expensive,
-	// if possible release the lock and re-acquire it.
+	// Slow path: load the InodeOperations into memory. Since this is a hot path and the lookup may be
+	// expensive, if possible release the lock and re-acquire it.
 	if walkMayUnlock {
+		// While this dirent is unlocked, the lookup below is not allowed to proceed in tandem with a
+		// rename operation. The rename should be fully complete before we call Lookup on anything.
 		d.mu.Unlock()
+		renameMu.RLock()
 	}
 	c, err := d.Inode.Lookup(ctx, name)
 	if walkMayUnlock {
 		d.mu.Lock()
+		renameMu.RUnlock()
 	}
 	// No dice.
 	if err != nil {
@@ -1047,34 +1051,12 @@ func (d *Dirent) flush() {
 	}
 }
 
-// Busy indicates whether this Dirent is a mount point or root dirent, or has
-// active positive children.
-//
-// This is expensive, since it flushes the children cache.
-//
-// TODO: Fix this busy-ness check.
+// Busy indicates whether this Dirent is a mount point or root dirent.
 func (d *Dirent) Busy() bool {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 
-	if d.mounted || d.parent == nil {
-		return true
-	}
-
-	// Flush any cached references to children that are doomed.
-	d.flush()
-
-	// Count positive children.
-	var nonNegative int
-	for _, w := range d.children {
-		if child := w.Get(); child != nil {
-			if !child.(*Dirent).IsNegative() {
-				nonNegative++
-			}
-			child.DecRef()
-		}
-	}
-	return nonNegative > 0
+	return d.mounted || d.parent == nil
 }
 
 // mount mounts a new dirent with the given inode over d.
-- 
cgit v1.2.3