From 2b0615c76c8acf88de1add9bad1758384fecb9e5 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Fri, 20 Aug 2021 17:46:45 -0700
Subject: [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
---
 runsc/fsgofer/fsgofer.go | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

(limited to 'runsc/fsgofer')

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
 }
-- 
cgit v1.2.3