[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

2 files changed, 15 insertions, 17 deletions

diff --git a/pkg/p9/file.go b/pkg/p9/file.go
index 97e0231d6..8d6af2d6b 100644
--- a/pkg/p9/file.go
+++ b/pkg/p9/file.go
@@ -325,6 +325,12 @@ func (*DisallowServerCalls) Renamed(File, string) {
 func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 	stats := make([]FullStat, 0, len(names))
 	parent := start
+	closeParent := func() {
+		if parent != start {
+			_ = parent.Close()
+		}
+	}
+	defer closeParent()
 	mask := AttrMaskAll()
 	for i, name := range names {
 		if len(name) == 0 && i == 0 {
@@ -340,15 +346,14 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 			continue
 		}
 		qids, child, valid, attr, err := parent.WalkGetAttr([]string{name})
-		if parent != start {
-			_ = parent.Close()
-		}
 		if err != nil {
 			if errors.Is(err, unix.ENOENT) {
 				return stats, nil
 			}
 			return nil, err
 		}
+		closeParent()
+		parent = child
 		stats = append(stats, FullStat{
 			QID:   qids[0],
 			Valid: valid,
@@ -357,13 +362,8 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 		if attr.Mode.FileType() != ModeDirectory {
 			// Doesn't need to continue if entry is not a dir. Including symlinks
 			// that cannot be followed.
-			_ = child.Close()
 			break
 		}
-		parent = child
-	}
-	if parent != start {
-		_ = parent.Close()
 	}
 	return stats, nil
 }
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
 }