add a flag to disable setpgid syscall

It is broken on some of our test systems.

4 files changed, 62 insertions, 51 deletions

diff --git a/executor/executor.cc b/executor/executor.cc
index 1ac518f3f..4d04c4fb9 100644
--- a/executor/executor.cc
+++ b/executor/executor.cc
@@ -68,6 +68,7 @@ bool flag_threaded;
 bool flag_collide;
 bool flag_deduplicate;
 bool flag_drop_privs;
+bool flag_no_setpgid;
 
 __attribute__((aligned(64 << 10))) char input_data[kMaxInput];
 __attribute__((aligned(64 << 10))) char output_data[kMaxOutput];
@@ -99,7 +100,7 @@ struct thread_t {
 	int num_args;
 	uint64_t args[kMaxArgs];
 	uint64_t res;
-	uint64_t errno;
+	uint64_t reserrno;
 	uint32_t cover_size;
 	int cover_fd;
 };
@@ -157,8 +158,9 @@ int main()
 	flag_collide = flags & (1 << 3);
 	flag_deduplicate = flags & (1 << 4);
 	flag_drop_privs = flags & (1 << 5);
-	if (flag_collide)
-		flag_threaded = true;
+	flag_no_setpgid = flags & (1 << 6);
+	if (!flag_threaded)
+		flag_collide = false;
 
 	cover_open();
 
@@ -175,7 +177,8 @@ int main()
 		if (pid < 0)
 			fail("fork failed");
 		if (pid == 0) {
-			setpgid(0, 0);
+			if (!flag_no_setpgid)
+				setpgid(0, 0);
 			unshare(CLONE_NEWNS);
 			close(kInPipeFd);
 			close(kOutPipeFd);
@@ -209,46 +212,48 @@ int main()
 		}
 
 		int status = 0;
-#if 1
-		timespec ts = {};
-		ts.tv_sec = 5;
-		ts.tv_nsec = 0;
-		if (sigtimedwait(&sigchldset, NULL, &ts) < 0) {
-			debug("sigtimedwait expired, killing %d\n", pid);
-			kill(-pid, SIGKILL);
-			kill(pid, SIGKILL);
-		}
-		debug("waitpid(%d)\n", pid);
-		if (waitpid(pid, &status, __WALL | WUNTRACED) != pid)
-			fail("waitpid failed");
-		debug("waitpid(%d) returned\n", pid);
-		// Drain SIGCHLD signals.
-		ts.tv_sec = 0;
-		ts.tv_nsec = 0;
-		while (sigtimedwait(&sigchldset, NULL, &ts) > 0) {
-		}
-#else
-		// This code is less efficient, but does not require working sigtimedwait.
-		// We've hit 2 systems that mishandle sigtimedwait.
-		uint64_t start = current_time_ms();
-		for (;;) {
-			int res = waitpid(pid, &status, __WALL | WUNTRACED | WNOHANG);
-			debug("waitpid(%d)=%d (%d)\n", pid, res, errno);
-			if (res == pid)
-				break;
-			usleep(1000);
-			if (current_time_ms() - start > 5 * 1000) {
-				debug("killing\n");
-				kill(-pid, SIGKILL);
+		if (!flag_no_setpgid) {
+			timespec ts = {};
+			ts.tv_sec = 5;
+			ts.tv_nsec = 0;
+			if (sigtimedwait(&sigchldset, NULL, &ts) < 0) {
+				debug("sigtimedwait expired, killing %d\n", pid);
+				if (!flag_no_setpgid)
+					kill(-pid, SIGKILL);
 				kill(pid, SIGKILL);
-				int res = waitpid(pid, &status, __WALL | WUNTRACED);
+			}
+			debug("waitpid(%d)\n", pid);
+			if (waitpid(pid, &status, __WALL | WUNTRACED) != pid)
+				fail("waitpid failed");
+			debug("waitpid(%d) returned\n", pid);
+			// Drain SIGCHLD signals.
+			ts.tv_sec = 0;
+			ts.tv_nsec = 0;
+			while (sigtimedwait(&sigchldset, NULL, &ts) > 0) {
+			}
+		}
+		else {
+			// This code is less efficient, but does not require working sigtimedwait.
+			// We've hit 2 systems that mishandle sigtimedwait.
+			uint64_t start = current_time_ms();
+			for (;;) {
+				int res = waitpid(pid, &status, __WALL | WUNTRACED | WNOHANG);
 				debug("waitpid(%d)=%d (%d)\n", pid, res, errno);
 				if (res == pid)
 					break;
-				fail("waitpid failed");
+				usleep(1000);
+				if (current_time_ms() - start > 5 * 1000) {
+					debug("killing\n");
+					kill(-pid, SIGKILL);
+					kill(pid, SIGKILL);
+					int res = waitpid(pid, &status, __WALL | WUNTRACED);
+					debug("waitpid(%d)=%d (%d)\n", pid, res, errno);
+					if (res == pid)
+						break;
+					fail("waitpid failed");
+				}
 			}
 		}
-#endif
 		status = WEXITSTATUS(status);
 		if (status == kFailStatus)
 			fail("child failed");
@@ -503,7 +508,7 @@ void handle_completion(thread_t* th)
 
 		write_output(th->call_index);
 		write_output(th->call_num);
-		write_output(th->res != -1 ? 0 : th->errno);
+		write_output(th->res != (uint64_t)-1 ? 0 : th->reserrno);
 		write_output(th->cover_size);
 		for (uint32_t i = 0; i < th->cover_size; i++)
 			write_output(th->cover_data[i + 1]);
@@ -558,7 +563,6 @@ void execute_call(thread_t* th)
 		if (th->num_args > 6)
 			fail("bad number of arguments");
 		th->res = syscall(call->sys_nr, th->args[0], th->args[1], th->args[2], th->args[3], th->args[4], th->args[5]);
-		th->errno = errno;
 		break;
 	}
 	case __NR_syz_openpts: {
@@ -572,14 +576,12 @@ void execute_call(thread_t* th)
 		else {
 			th->res = -1;
 		}
-		th->errno = errno;
 	}
 	case __NR_syz_dri_open: {
 		// syz_dri_open(card_id intptr, flags flags[open_flags]) fd[dri]
 		char buf[128];
 		sprintf(buf, "/dev/dri/card%lu", th->args[0]);
 		th->res = open(buf, th->args[1], 0);
-		th->errno = errno;
 	}
 	case __NR_syz_fuse_mount: {
 		// syz_fuse_mount(target filename, mode flags[fuse_mode], uid uid, gid gid, maxread intptr, flags flags[mount_flags]) fd[fuse]
@@ -604,7 +606,6 @@ void execute_call(thread_t* th)
 			// Ignore errors, maybe fuzzer can do something useful with fd alone.
 		}
 		th->res = fd;
-		th->errno = errno;
 	}
 	case __NR_syz_fuseblk_mount: {
 		// syz_fuseblk_mount(target filename, blkdev filename, mode flags[fuse_mode], uid uid, gid gid, maxread intptr, blksize intptr, flags flags[mount_flags]) fd[fuse]
@@ -635,14 +636,13 @@ void execute_call(thread_t* th)
 			}
 		}
 		th->res = fd;
-		th->errno = errno;
 	}
 	}
-	int errno0 = errno;
+	th->reserrno = errno;
 	th->cover_size = cover_read(th);
 
 	if (th->res == (uint64_t)-1)
-		debug("#%d: %s = errno(%d)\n", th->id, call->name, errno0);
+		debug("#%d: %s = errno(%d)\n", th->id, call->name, th->reserrno);
 	else
 		debug("#%d: %s = 0x%lx\n", th->id, call->name, th->res);
 	__atomic_store_n(&th->done, 1, __ATOMIC_RELEASE);
diff --git a/ipc/ipc.go b/ipc/ipc.go
index 876d75cde..7e4ba7ca1 100644
--- a/ipc/ipc.go
+++ b/ipc/ipc.go
@@ -41,6 +41,7 @@ const (
 	FlagCollide                        // collide syscalls to provoke data races
 	FlagDedupCover                     // deduplicate coverage in executor
 	FlagDropPrivs                      // impersonate nobody user
+	FlagNoSetpgid                      // don't use setpgid
 	FlagStrace                         // run executor under strace
 )
 
@@ -252,6 +253,7 @@ func closeMapping(f *os.File, mem []byte) error {
 type command struct {
 	timeout time.Duration
 	cmd     *exec.Cmd
+	flags   uint64
 	dir     string
 	rp      *os.File
 	inrp    *os.File
@@ -264,7 +266,7 @@ func makeCommand(bin []string, timeout time.Duration, flags uint64, inFile *os.F
 		return nil, fmt.Errorf("failed to create temp dir: %v", err)
 	}
 
-	c := &command{timeout: timeout, dir: dir}
+	c := &command{timeout: timeout, flags: flags, dir: dir}
 	defer func() {
 		if c != nil {
 			c.close()
@@ -330,11 +332,10 @@ func makeCommand(bin []string, timeout time.Duration, flags uint64, inFile *os.F
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stdout
 	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: flags&FlagNoSetpgid == 0}
 	if syscall.Getuid() == 0 {
 		// Running under root, more isolation is possible.
-		cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Cloneflags: syscall.CLONE_NEWNS}
-	} else {
-		cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+		cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWNS
 	}
 	if err := cmd.Start(); err != nil {
 		return nil, fmt.Errorf("failed to start executor binary: %v", err)
@@ -366,7 +367,9 @@ func (c *command) kill() {
 	// We started the process in its own process group and now kill the whole group.
 	// This solves a potential problem with strace:
 	// if we kill just strace, executor still runs and ReadAll below hangs.
-	syscall.Kill(-c.cmd.Process.Pid, syscall.SIGKILL)
+	if c.flags&FlagNoSetpgid == 0 {
+		syscall.Kill(-c.cmd.Process.Pid, syscall.SIGKILL)
+	}
 	syscall.Kill(c.cmd.Process.Pid, syscall.SIGKILL)
 }
 
diff --git a/tools/syz-execprog/execprog.go b/tools/syz-execprog/execprog.go
index a94b87878..487b6e0d6 100644
--- a/tools/syz-execprog/execprog.go
+++ b/tools/syz-execprog/execprog.go
@@ -34,6 +34,7 @@ var (
 	flagDedup     = flag.Bool("dedup", false, "deduplicate coverage in executor")
 	flagLoop      = flag.Bool("loop", false, "execute programs in a loop")
 	flagProcs     = flag.Int("procs", 1, "number of parallel processes to execute programs")
+	flagNoPgid    = flag.Bool("nopgid", false, "don't use setpgid syscall")
 	flagTimeout   = flag.Duration("timeout", 10*time.Second, "execution timeout")
 )
 
@@ -76,6 +77,9 @@ func main() {
 	if *flagNobody {
 		flags |= ipc.FlagDropPrivs
 	}
+	if *flagNoPgid {
+		flags |= ipc.FlagNoSetpgid
+	}
 
 	var wg sync.WaitGroup
 	wg.Add(*flagProcs)
diff --git a/tools/syz-stress/stress.go b/tools/syz-stress/stress.go
index da013507f..2a30dd6ac 100644
--- a/tools/syz-stress/stress.go
+++ b/tools/syz-stress/stress.go
@@ -30,6 +30,7 @@ var (
 	flagThreaded = flag.Bool("threaded", true, "use threaded mode in executor")
 	flagCollide  = flag.Bool("collide", true, "collide syscalls to provoke data races")
 	flagNobody   = flag.Bool("nobody", true, "impersonate into nobody")
+	flagNoPgid   = flag.Bool("nopgid", false, "don't use setpgid syscall")
 	flagTimeout  = flag.Duration("timeout", 10*time.Second, "executor timeout")
 	flagLogProg  = flag.Bool("logprog", false, "print programs before execution")
 
@@ -58,6 +59,9 @@ func main() {
 	if *flagDebug {
 		flags |= ipc.FlagDebug
 	}
+	if *flagNoPgid {
+		flags |= ipc.FlagNoSetpgid
+	}
 
 	gate = ipc.NewGate(2 * *flagProcs)
 	for pid := 0; pid < *flagProcs; pid++ {