summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorDean Deng <deandeng@google.com>2019-10-29 10:03:18 -0700
committergVisor bot <gvisor-bot@google.com>2019-10-29 10:04:39 -0700
commit29273b03842a85bce8314799348231520ceb6e9c (patch)
tree118b42eb7eb8ef0f6942ca75f1575de1fdf272ee
parentdbeaf9d4dbeea4cde670c3d07a78b56a45fa8f21 (diff)
Disallow execveat on interpreter scripts with fd opened with O_CLOEXEC.
When an interpreter script is opened with O_CLOEXEC and the resulting fd is passed into execveat, an ENOENT error should occur (the script would otherwise be inaccessible to the interpreter). This matches the actual behavior of Linux's execveat. PiperOrigin-RevId: 277306680
-rw-r--r--pkg/sentry/kernel/kernel.go1
-rw-r--r--pkg/sentry/loader/loader.go9
-rw-r--r--pkg/sentry/syscalls/linux/sys_thread.go5
-rw-r--r--test/syscalls/linux/exec.cc33
4 files changed, 47 insertions, 1 deletions
diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go
index fcfe7a16d..e64d648e2 100644
--- a/pkg/sentry/kernel/kernel.go
+++ b/pkg/sentry/kernel/kernel.go
@@ -812,6 +812,7 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID,
ResolveFinal: true,
Filename: args.Filename,
File: args.File,
+ CloseOnExec: false,
Argv: args.Argv,
Envv: args.Envv,
Features: k.featureSet,
diff --git a/pkg/sentry/loader/loader.go b/pkg/sentry/loader/loader.go
index 818941762..f75ebe08a 100644
--- a/pkg/sentry/loader/loader.go
+++ b/pkg/sentry/loader/loader.go
@@ -66,6 +66,12 @@ type LoadArgs struct {
// nil, then File will be loaded and Filename will be ignored.
File *fs.File
+ // CloseOnExec indicates that the executable (or one of its parent
+ // directories) was opened with O_CLOEXEC. If the executable is an
+ // interpreter script, then cause an ENOENT error to occur, since the
+ // script would otherwise be inaccessible to the interpreter.
+ CloseOnExec bool
+
// Argv is the vector of arguments to pass to the executable.
Argv []string
@@ -279,6 +285,9 @@ func loadExecutable(ctx context.Context, args LoadArgs) (loadedELF, arch.Context
d.IncRef()
return loaded, ac, d, args.Argv, err
case bytes.Equal(hdr[:2], []byte(interpreterScriptMagic)):
+ if args.CloseOnExec {
+ return loadedELF{}, nil, nil, nil, syserror.ENOENT
+ }
args.Filename, args.Argv, err = parseInterpreterScript(ctx, args.Filename, args.File, args.Argv)
if err != nil {
ctx.Infof("Error loading interpreter script: %v", err)
diff --git a/pkg/sentry/syscalls/linux/sys_thread.go b/pkg/sentry/syscalls/linux/sys_thread.go
index 2476f8858..4115116ff 100644
--- a/pkg/sentry/syscalls/linux/sys_thread.go
+++ b/pkg/sentry/syscalls/linux/sys_thread.go
@@ -120,6 +120,7 @@ func execveat(t *kernel.Task, dirFD int32, pathnameAddr, argvAddr, envvAddr user
var wd *fs.Dirent
var executable *fs.File
+ var closeOnExec bool
if dirFD == linux.AT_FDCWD || path.IsAbs(pathname) {
// Even if the pathname is absolute, we may still need the wd
// for interpreter scripts if the path of the interpreter is
@@ -127,11 +128,12 @@ func execveat(t *kernel.Task, dirFD int32, pathnameAddr, argvAddr, envvAddr user
wd = t.FSContext().WorkingDirectory()
} else {
// Need to extract the given FD.
- f := t.GetFile(dirFD)
+ f, fdFlags := t.FDTable().Get(dirFD)
if f == nil {
return 0, nil, syserror.EBADF
}
defer f.DecRef()
+ closeOnExec = fdFlags.CloseOnExec
if atEmptyPath && len(pathname) == 0 {
executable = f
@@ -157,6 +159,7 @@ func execveat(t *kernel.Task, dirFD int32, pathnameAddr, argvAddr, envvAddr user
ResolveFinal: resolveFinal,
Filename: pathname,
File: executable,
+ CloseOnExec: closeOnExec,
Argv: argv,
Envv: envv,
Features: t.Arch().FeatureSet(),
diff --git a/test/syscalls/linux/exec.cc b/test/syscalls/linux/exec.cc
index 21a5ffd40..a9067df2a 100644
--- a/test/syscalls/linux/exec.cc
+++ b/test/syscalls/linux/exec.cc
@@ -681,6 +681,39 @@ TEST(ExecveatTest, SymlinkNoFollowWithNormalFile) {
ArgEnvExitStatus(0, 0), "");
}
+TEST(ExecveatTest, BasicWithCloexecFD) {
+ std::string path = WorkloadPath(kBasicWorkload);
+ const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_CLOEXEC));
+
+ CheckExecveat(fd.get(), "", {path}, {}, AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH,
+ ArgEnvExitStatus(0, 0), absl::StrCat(path, "\n"));
+}
+
+TEST(ExecveatTest, InterpreterScriptWithCloexecFD) {
+ std::string path = WorkloadPath(kExitScript);
+ const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_CLOEXEC));
+
+ int execve_errno;
+ ASSERT_NO_ERRNO_AND_VALUE(ForkAndExecveat(fd.get(), "", {path}, {},
+ AT_EMPTY_PATH, /*child=*/nullptr,
+ &execve_errno));
+ EXPECT_EQ(execve_errno, ENOENT);
+}
+
+TEST(ExecveatTest, InterpreterScriptWithCloexecDirFD) {
+ std::string absolute_path = WorkloadPath(kExitScript);
+ std::string parent_dir = std::string(Dirname(absolute_path));
+ std::string base = std::string(Basename(absolute_path));
+ const FileDescriptor dirfd =
+ ASSERT_NO_ERRNO_AND_VALUE(Open(parent_dir, O_CLOEXEC | O_DIRECTORY));
+
+ int execve_errno;
+ ASSERT_NO_ERRNO_AND_VALUE(ForkAndExecveat(dirfd.get(), base, {base}, {},
+ /*flags=*/0, /*child=*/nullptr,
+ &execve_errno));
+ EXPECT_EQ(execve_errno, ENOENT);
+}
+
TEST(ExecveatTest, InvalidFlags) {
int execve_errno;
ASSERT_NO_ERRNO_AND_VALUE(ForkAndExecveat(