Return EACCES when calling setpgid() after execve()

From setpgid manpage,

EACCES - An attempt was made to change the process group ID of one
of the children of the calling process and the child had
already performed an execve(2) (setpgid(), setpgrp()).

This CL makes gVisor implement this rule and updates the exec test
suite accordingly.

TESTED: http://sponge2/7f364e8a-4f82-463e-ba62-79234c4d054d
PiperOrigin-RevId: 727095560

pkg/sentry/kernel/sessions.go

@@ -431,13 +431,9 @@ func (tg *ThreadGroup) CreateProcessGroup() error {
 
 // JoinProcessGroup joins an existing process group.
 //
-// This function will return EACCES if an exec has been performed since fork
-// by the given ThreadGroup, and EPERM if the Sessions are not the same or the
+// This function will return EPERM if the Sessions are not the same or the
 // group does not exist.
-//
-// If checkExec is set, then the join is not permitted after the process has
-// executed exec at least once.
-func (tg *ThreadGroup) JoinProcessGroup(pidns *PIDNamespace, pgid ProcessGroupID, checkExec bool) error {
+func (tg *ThreadGroup) JoinProcessGroup(pidns *PIDNamespace, pgid ProcessGroupID) error {
 	pidns.owner.mu.Lock()
 	defer pidns.owner.mu.Unlock()
 
@@ -452,11 +448,6 @@ func (tg *ThreadGroup) JoinProcessGroup(pidns *PIDNamespace, pgid ProcessGroupID
 		return linuxerr.EPERM
 	}
 
-	// Disallow the join if an execve has performed, per POSIX.
-	if checkExec && tg.execed {
-		return linuxerr.EACCES
-	}
-
 	// See if it's in the same session as ours.
 	if pg.session != tg.processGroup.session {
 		return linuxerr.EPERM

pkg/sentry/kernel/task_exec.go

@@ -205,9 +205,9 @@ func (r *runSyscallAfterExecStop) execute(t *Task) taskRunState {
 	t.signalStack = linux.SignalStack{Flags: linux.SS_DISABLE}
 	// "The termination signal is reset to SIGCHLD (see clone(2))."
 	t.tg.terminationSignal = linux.SIGCHLD
-	// execed indicates that the process can no longer join a process group
+	// execed indicates that the process's pgid cannot be changed
 	// in some scenarios (namely, the parent call setpgid(2) on the child).
-	// See the JoinProcessGroup function in sessions.go for more context.
+	// See the Setpgid function in sys_thread.go for more context.
 	t.tg.execed = true
 	// Maximum RSS is preserved across execve(2).
 	t.updateRSSLocked()

pkg/sentry/kernel/thread_group.go

@@ -651,3 +651,11 @@ func (tg *ThreadGroup) IsInitIn(pidns *PIDNamespace) bool {
 func (tg *ThreadGroup) isInitInLocked(pidns *PIDNamespace) bool {
 	return pidns.tgids[tg] == initTID
 }
+
+// Execed returns whether this ThreadGroup has execed since creation.
+func (tg *ThreadGroup) Execed() bool {
+	ts := tg.TaskSet()
+	ts.mu.RLock()
+	defer ts.mu.RUnlock()
+	return tg.execed
+}

pkg/sentry/syscalls/linux/sys_thread.go

@@ -620,6 +620,13 @@ func Setpgid(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr
 		return 0, nil, linuxerr.EINVAL
 	}
 
+	// Return EACCES if an attempt was made to change the process group ID of one
+	// of the children of the calling process and the child had
+	// already performed an execve(2)
+	if tg != t.ThreadGroup() && tg.Execed() {
+		return 0, nil, linuxerr.EACCES
+	}
+
 	// If the pgid is the same as the group, then create a new one. Otherwise,
 	// we attempt to join an existing process group.
 	if pgid == defaultPGID {
@@ -635,7 +642,7 @@ func Setpgid(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr
 		}
 	} else {
 		// Same as CreateProcessGroup, above.
-		if err := tg.JoinProcessGroup(t.PIDNamespace(), pgid, tg != t.ThreadGroup()); err != nil {
+		if err := tg.JoinProcessGroup(t.PIDNamespace(), pgid); err != nil {
 			// See above.
 			if t.PIDNamespace().IDOfProcessGroup(tg.ProcessGroup()) == pgid {
 				return 0, nil, nil

test/syscalls/linux/exec.cc

@@ -17,18 +17,22 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/eventfd.h>
+#include <sys/ptrace.h>
 #include <sys/resource.h>
 #include <sys/time.h>
 #include <unistd.h>
 
+#include <cassert>
+#include <csignal>
+#include <cstdint>
+#include <cstdlib>
 #include <iostream>
-#include <memory>
 #include <string>
 #include <vector>
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "absl/strings/match.h"
-#include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
@@ -813,6 +817,40 @@ TEST(GetpriorityTest, ExecveMaintainsPriority) {
             {}, W_EXITCODE(expected_exit_code, 0), "");
 }
 
+// Test that setpgid() fails on child processes after they call execve().
+TEST(ExecTest, Setpgid) {
+  const pid_t pid = fork();
+  int status;
+  ASSERT_NE(pid, -1);
+  if (pid == 0) {
+    ASSERT_THAT(ptrace(PTRACE_TRACEME, 0, 0, 0), SyscallSucceeds());
+    raise(SIGSTOP);
+    char* argv[] = {nullptr};
+    char* envp[] = {nullptr};
+    ASSERT_THAT(execve("/proc/self/exe", argv, envp), SyscallSucceeds());
+  }
+
+  EXPECT_THAT(setpgid(pid, pid), SyscallSucceeds())
+      << "setpgid failed before execve";
+  ASSERT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid))
+      << "waitpid failed";
+  ASSERT_THAT(WIFSTOPPED(status), 1);
+  ASSERT_THAT(WSTOPSIG(status), SIGSTOP);
+  ASSERT_THAT(
+      ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL | PTRACE_O_TRACEEXEC),
+      SyscallSucceeds())
+      << "ptrace failed";
+  ASSERT_THAT(ptrace(PTRACE_CONT, pid, 0, 0), SyscallSucceeds())
+      << "ptrace (PTRACE_CONT) failed";
+  ASSERT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid))
+      << "waitpid failed";
+  ASSERT_THAT(WIFSTOPPED(status), 1);
+  ASSERT_THAT(WSTOPSIG(status), SIGTRAP);
+  EXPECT_THAT(setpgid(pid, pid), SyscallFailsWithErrno(EACCES));
+  EXPECT_THAT(setpgid(pid, getpid()), SyscallFailsWithErrno(EACCES));
+  EXPECT_THAT(setpgid(getpid(), pid), SyscallSucceeds());
+}
+
 void ExecWithThread() {
   // Used to ensure that the thread has actually started.
   absl::Mutex mu;