From 24d8656585e6072ff7d5a00a7eb4bd25cba42dc4 Mon Sep 17 00:00:00 2001
From: Andrei Vagin <avagin@google.com>
Date: Fri, 3 May 2019 14:00:31 -0700
Subject: [PATCH] gofer: don't leak file descriptors

Fixes #219

PiperOrigin-RevId: 246568639
Change-Id: Ic7afd15dde922638d77f6429c508d1cbe2e4288a
---
 pkg/sentry/fs/gofer/cache_policy.go | 3 ++-
 pkg/sentry/fs/gofer/path.go         | 4 ++++
 runsc/fsgofer/fsgofer.go            | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/pkg/sentry/fs/gofer/cache_policy.go b/pkg/sentry/fs/gofer/cache_policy.go
index 35cd0c1d6..c59344589 100644
--- a/pkg/sentry/fs/gofer/cache_policy.go
+++ b/pkg/sentry/fs/gofer/cache_policy.go
@@ -139,11 +139,12 @@ func (cp cachePolicy) revalidate(ctx context.Context, name string, parent, child
 	// TODO(b/112031682): If we have a directory FD in the parent
 	// inodeOperations, then we can use fstatat(2) to get the inode
 	// attributes instead of making this RPC.
-	qids, _, mask, attr, err := parentIops.fileState.file.walkGetAttr(ctx, []string{name})
+	qids, f, mask, attr, err := parentIops.fileState.file.walkGetAttr(ctx, []string{name})
 	if err != nil {
 		// Can't look up the name. Trigger reload.
 		return true
 	}
+	f.close(ctx)
 
 	// If the Path has changed, then we are not looking at the file file.
 	// We must reload.
diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go
index 4cbf9e9d9..aa3d3aaa6 100644
--- a/pkg/sentry/fs/gofer/path.go
+++ b/pkg/sentry/fs/gofer/path.go
@@ -109,6 +109,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
 	hostFile, err := newFile.create(ctx, name, openFlags, p9.FileMode(perm.LinuxMode()), p9.UID(owner.UID), p9.GID(owner.GID))
 	if err != nil {
 		// Could not create the file.
+		newFile.close(ctx)
 		return nil, err
 	}
 
@@ -120,11 +121,14 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
 	qids, unopened, mask, p9attr, err := i.fileState.file.walkGetAttr(ctx, []string{name})
 	if err != nil {
 		newFile.close(ctx)
+		hostFile.Close()
 		return nil, err
 	}
 	if len(qids) != 1 {
 		log.Warningf("WalkGetAttr(%s) succeeded, but returned %d QIDs (%v), wanted 1", name, len(qids), qids)
 		newFile.close(ctx)
+		hostFile.Close()
+		unopened.close(ctx)
 		return nil, syserror.EIO
 	}
 	qid := qids[0]
diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index 158f22ddc..3a0806837 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -502,6 +502,9 @@ func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
 	last := l
 	for _, name := range names {
 		f, path, err := openAnyFileFromParent(last, name)
+		if last != l {
+			last.Close()
+		}
 		if err != nil {
 			return nil, nil, extractErrno(err)
 		}