Imported FD in exec was leaking

Imported file needs to be closed after it's
been imported.

PiperOrigin-RevId: 211732472
Change-Id: Ia9249210558b77be076bcce465b832a22eed301f

4 files changed, 86 insertions, 3 deletions

diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go
index 2493c5175..4848a5d2b 100644
--- a/pkg/sentry/control/proc.go
+++ b/pkg/sentry/control/proc.go
@@ -119,6 +119,10 @@ func (proc *Proc) Exec(args *ExecArgs, waitStatus *uint32) error {
 			return err
 		}
 		defer file.DecRef()
+
+		// We're done with this file.
+		f.Close()
+
 		if err := fdm.NewFDAt(kdefs.FD(appFD), file, kernel.FDFlags{}, l); err != nil {
 			return err
 		}
diff --git a/pkg/sentry/fs/host/BUILD b/pkg/sentry/fs/host/BUILD
index f1252b0f2..d1a6eaf6e 100644
--- a/pkg/sentry/fs/host/BUILD
+++ b/pkg/sentry/fs/host/BUILD
@@ -55,6 +55,7 @@ go_test(
     name = "host_test",
     size = "small",
     srcs = [
+        "descriptor_test.go",
         "fs_test.go",
         "inode_test.go",
         "socket_test.go",
diff --git a/pkg/sentry/fs/host/descriptor.go b/pkg/sentry/fs/host/descriptor.go
index 3aee4d11c..148291ba6 100644
--- a/pkg/sentry/fs/host/descriptor.go
+++ b/pkg/sentry/fs/host/descriptor.go
@@ -31,9 +31,9 @@ type descriptor struct {
 	// donated is true if the host fd was donated by another process.
 	donated bool
 
-	// If origFD >= 0, it is the host fd that this file was
-	// originally created from, which must be available at time
-	// of restore. Only valid if donated is true.
+	// If origFD >= 0, it is the host fd that this file was originally created
+	// from, which must be available at time of restore. The FD can be closed
+	// after descriptor is created. Only set if donated is true.
 	origFD int
 
 	// wouldBlock is true if value (below) points to a file that can
diff --git a/pkg/sentry/fs/host/descriptor_test.go b/pkg/sentry/fs/host/descriptor_test.go
new file mode 100644
index 000000000..f393a8b54
--- /dev/null
+++ b/pkg/sentry/fs/host/descriptor_test.go
@@ -0,0 +1,78 @@
+// Copyright 2018 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package host
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"gvisor.googlesource.com/gvisor/pkg/waiter"
+	"gvisor.googlesource.com/gvisor/pkg/waiter/fdnotifier"
+)
+
+func TestDescriptorRelease(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		saveable   bool
+		wouldBlock bool
+	}{
+		{name: "all false"},
+		{name: "saveable", saveable: true},
+		{name: "wouldBlock", wouldBlock: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dir, err := ioutil.TempDir("", "descriptor_test")
+			if err != nil {
+				t.Fatal("ioutil.TempDir() failed:", err)
+			}
+
+			fd, err := syscall.Open(filepath.Join(dir, "file"), syscall.O_RDWR|syscall.O_CREAT, 0666)
+			if err != nil {
+				t.Fatal("failed to open temp file:", err)
+			}
+
+			// FD ownership is transferred to the descritor.
+			queue := &waiter.Queue{}
+			d, err := newDescriptor(fd, false /* donated*/, tc.saveable, tc.wouldBlock, queue)
+			if err != nil {
+				syscall.Close(fd)
+				t.Fatalf("newDescriptor(%d, %t, false, %t, queue) failed, err: %v", fd, tc.saveable, tc.wouldBlock, err)
+			}
+			if tc.saveable {
+				if d.origFD < 0 {
+					t.Errorf("saveable descriptor must preserve origFD, desc: %+v", d)
+				}
+			}
+			if tc.wouldBlock {
+				if !fdnotifier.HasFD(int32(d.value)) {
+					t.Errorf("FD not registered with notifier, desc: %+v", d)
+				}
+			}
+
+			oldVal := d.value
+			d.Release()
+			if d.value != -1 {
+				t.Errorf("d.value want: -1, got: %d", d.value)
+			}
+			if tc.wouldBlock {
+				if fdnotifier.HasFD(int32(oldVal)) {
+					t.Errorf("FD not unregistered with notifier, desc: %+v", d)
+				}
+			}
+		})
+	}
+}