[op] Prevent file leak in MultiGetAttr's error path.

The old implementation was mostly correct but error prone - making way for the
issue in question here. In its error path, it would leak the intermediate file
being walked. Each return/break needed explicit cleanup.

This change implements a more clean way to cleaning up intermediate directories.
If the code were to evolve to be more complex, it would still work.

PiperOrigin-RevId: 392102826

1 files changed, 7 insertions, 9 deletions

diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index 07497e47b..600b21189 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -1242,13 +1242,14 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
 	}
 
 	parent := l.file.FD()
-	for _, name := range names {
-		child, err := unix.Openat(parent, name, openFlags|unix.O_PATH, 0)
+	closeParent := func() {
 		if parent != l.file.FD() {
-			// Parent is no longer needed.
 			_ = unix.Close(parent)
-			parent = -1
 		}
+	}
+	defer closeParent()
+	for _, name := range names {
+		child, err := unix.Openat(parent, name, openFlags|unix.O_PATH, 0)
 		if err != nil {
 			if errors.Is(err, unix.ENOENT) {
 				// No pont in continuing any further.
@@ -1256,10 +1257,11 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
 			}
 			return nil, err
 		}
+		closeParent()
+		parent = child
 
 		var stat unix.Stat_t
 		if err := unix.Fstat(child, &stat); err != nil {
-			_ = unix.Close(child)
 			return nil, err
 		}
 		valid, attr := l.fillAttr(&stat)
@@ -1271,13 +1273,9 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) {
 		if (stat.Mode & unix.S_IFMT) != unix.S_IFDIR {
 			// Doesn't need to continue if entry is not a dir. Including symlinks
 			// that cannot be followed.
-			_ = unix.Close(child)
 			break
 		}
 		parent = child
 	}
-	if parent != -1 && parent != l.file.FD() {
-		_ = unix.Close(parent)
-	}
 	return stats, nil
 }