Date: Wed, 17 Mar 2021 22:23:05 GMT From: Alex Richardson <arichardson@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 60a9c919a532 - stable/13 - Update capsicum-test to git commit f4d97414d48b8f8356b971ab9f45dc5c70d53c40 Message-ID: <202103172223.12HMN5Yb045968@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by arichardson: URL: https://cgit.FreeBSD.org/src/commit/?id=60a9c919a53273a1535671f39084913b4eff95c5 commit 60a9c919a53273a1535671f39084913b4eff95c5 Author: Alex Richardson <arichardson@FreeBSD.org> AuthorDate: 2021-03-02 16:28:26 +0000 Commit: Alex Richardson <arichardson@FreeBSD.org> CommitDate: 2021-03-17 22:22:49 +0000 Update capsicum-test to git commit f4d97414d48b8f8356b971ab9f45dc5c70d53c40 This includes various fixes that I submitted recently such as updating the pdkill() tests for the actual implemented behaviour (https://github.com/google/capsicum-test/pull/53) and lots of changes to avoid calling sleep() and replacing it with reliable synchronization (pull requests 49,51,52,53,54). This should make the testsuite more reliable when running on Jenkins. Additionally, process status is now retrieved using libprocstat instead of running `ps` and parsing the output (https://github.com/google/capsicum-test/pull/50). This fixes one previously failing test and speeds up execution. Overall, this update reduces the total runtime from ~60s to about 4-5 seconds. (cherry picked from commit 955a3f9ad586f38395e66127f9f2f4afbf3d5a94) --- contrib/capsicum-test/GNUmakefile | 4 + contrib/capsicum-test/capability-fd.cc | 23 +++- contrib/capsicum-test/capmode.cc | 126 +++++++++++++---- contrib/capsicum-test/capsicum-test.cc | 88 ++++++++---- contrib/capsicum-test/capsicum-test.h | 36 ++++- contrib/capsicum-test/makefile | 2 +- contrib/capsicum-test/openat.cc | 1 + contrib/capsicum-test/procdesc.cc | 241 ++++++++++++++++++++++++--------- contrib/capsicum-test/socket.cc | 18 ++- tests/sys/capsicum/Makefile | 2 +- 10 files changed, 412 insertions(+), 129 deletions(-) diff --git a/contrib/capsicum-test/GNUmakefile b/contrib/capsicum-test/GNUmakefile index d7133ca3b386..426eb49cdfa1 100644 --- a/contrib/capsicum-test/GNUmakefile +++ b/contrib/capsicum-test/GNUmakefile @@ -4,6 +4,10 @@ OS:=$(shell uname) ARCH?=64 ARCHFLAG=-m$(ARCH) +ifeq ($(OS),FreeBSD) +EXTRA_LIBS=-lprocstat +endif + ifeq ($(OS),Linux) PROCESSOR:=$(shell uname -p) diff --git a/contrib/capsicum-test/capability-fd.cc b/contrib/capsicum-test/capability-fd.cc index a454d54aa86a..f255c6425cdd 100644 --- a/contrib/capsicum-test/capability-fd.cc +++ b/contrib/capsicum-test/capability-fd.cc @@ -486,6 +486,7 @@ FORK_TEST_ON(Capability, Mmap, TmpFile("cap_mmap_operations")) { // Given a file descriptor, create a capability with specific rights and // make sure only those rights work. #define TRY_FILE_OPS(fd, ...) do { \ + SCOPED_TRACE(#__VA_ARGS__); \ cap_rights_t rights; \ cap_rights_init(&rights, __VA_ARGS__); \ TryFileOps((fd), rights); \ @@ -1019,6 +1020,8 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_fd_transfer")) { if (child == 0) { // Child: enter cap mode EXPECT_OK(cap_enter()); + // Child: send startup notification + SEND_INT_MESSAGE(sock_fds[0], MSG_CHILD_STARTED); // Child: wait to receive FD over socket int rc = recvmsg(sock_fds[0], &mh, 0); @@ -1036,10 +1039,12 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_fd_transfer")) { EXPECT_RIGHTS_EQ(&r_rs, &rights); TryReadWrite(cap_fd); + // Child: acknowledge that we have received and tested the file descriptor + SEND_INT_MESSAGE(sock_fds[0], MSG_CHILD_FD_RECEIVED); + // Child: wait for a normal read - int val; - read(sock_fds[0], &val, sizeof(val)); - exit(0); + AWAIT_INT_MESSAGE(sock_fds[0], MSG_PARENT_REQUEST_CHILD_EXIT); + exit(testing::Test::HasFailure()); } int fd = open(TmpFile("cap_fd_transfer"), O_RDWR | O_CREAT, 0644); @@ -1054,6 +1059,9 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_fd_transfer")) { // Confirm we can do the right operations on the capability TryReadWrite(cap_fd); + // Wait for child to start up: + AWAIT_INT_MESSAGE(sock_fds[1], MSG_CHILD_STARTED); + // Send the file descriptor over the pipe to the sub-process mh.msg_controllen = CMSG_LEN(sizeof(int)); cmptr = CMSG_FIRSTHDR(&mh); @@ -1063,13 +1071,14 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_fd_transfer")) { *(int *)CMSG_DATA(cmptr) = cap_fd; buffer1[0] = 0; iov[0].iov_len = 1; - sleep(3); int rc = sendmsg(sock_fds[1], &mh, 0); EXPECT_OK(rc); - sleep(1); // Ensure subprocess runs - int zero = 0; - write(sock_fds[1], &zero, sizeof(zero)); + // Check that the child received the message + AWAIT_INT_MESSAGE(sock_fds[1], MSG_CHILD_FD_RECEIVED); + + // Tell the child to exit + SEND_INT_MESSAGE(sock_fds[1], MSG_PARENT_REQUEST_CHILD_EXIT); } TEST(Capability, SyscallAt) { diff --git a/contrib/capsicum-test/capmode.cc b/contrib/capsicum-test/capmode.cc index 567773f319d9..c274f5e1c9f3 100644 --- a/contrib/capsicum-test/capmode.cc +++ b/contrib/capsicum-test/capmode.cc @@ -528,6 +528,8 @@ TEST(Capmode, Abort) { FORK_TEST_F(WithFiles, AllowedMiscSyscalls) { umask(022); mode_t um_before = umask(022); + int pipefds[2]; + EXPECT_OK(pipe(pipefds)); EXPECT_OK(cap_enter()); // Enter capability mode. mode_t um = umask(022); @@ -540,13 +542,19 @@ FORK_TEST_F(WithFiles, AllowedMiscSyscalls) { pid_t pid = fork(); EXPECT_OK(pid); if (pid == 0) { - // Child: almost immediately exit. - sleep(1); + // Child: wait for an exit message from parent (so we can test waitpid). + EXPECT_OK(close(pipefds[0])); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); + AWAIT_INT_MESSAGE(pipefds[1], MSG_PARENT_REQUEST_CHILD_EXIT); exit(0); } else if (pid > 0) { + EXPECT_OK(close(pipefds[1])); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); errno = 0; EXPECT_CAPMODE(ptrace_(PTRACE_PEEKDATA_, pid, &pid, NULL)); - EXPECT_CAPMODE(waitpid(pid, NULL, 0)); + EXPECT_CAPMODE(waitpid(pid, NULL, WNOHANG)); + SEND_INT_MESSAGE(pipefds[0], MSG_PARENT_REQUEST_CHILD_EXIT); + if (verbose) fprintf(stderr, " child finished\n"); } // No error return from sync(2) to test, but check errno remains unset. @@ -568,68 +576,136 @@ FORK_TEST_F(WithFiles, AllowedMiscSyscalls) { } void *thread_fn(void *p) { - int delay = *(int *)p; - sleep(delay); + int fd = (int)(intptr_t)p; + if (verbose) fprintf(stderr, " thread waiting to run\n"); + AWAIT_INT_MESSAGE(fd, MSG_PARENT_CHILD_SHOULD_RUN); EXPECT_OK(getpid_()); EXPECT_CAPMODE(open("/dev/null", O_RDWR)); - return NULL; + // Return whether there have been any failures to the main thread. + void *rval = (void *)(intptr_t)testing::Test::HasFailure(); + if (verbose) fprintf(stderr, " thread finished: %p\n", rval); + return rval; } // Check that restrictions are the same in subprocesses and threads FORK_TEST(Capmode, NewThread) { // Fire off a new thread before entering capability mode pthread_t early_thread; - int one = 1; // second - EXPECT_OK(pthread_create(&early_thread, NULL, thread_fn, &one)); + void *thread_rval; + // Create two pipes, one for synchronization with the threads, the other to + // synchronize with the children (since we can't use waitpid after cap_enter). + // Note: Could use pdfork+pdwait instead, but that is tested in procdesc.cc. + int thread_pipe[2]; + EXPECT_OK(pipe(thread_pipe)); + int proc_pipe[2]; + EXPECT_OK(pipe(proc_pipe)); + EXPECT_OK(pthread_create(&early_thread, NULL, thread_fn, + (void *)(intptr_t)thread_pipe[1])); // Fire off a new process before entering capability mode. + if (verbose) fprintf(stderr, " starting second child (non-capability mode)\n"); int early_child = fork(); EXPECT_OK(early_child); if (early_child == 0) { - // Child: wait and then confirm this process is unaffect by capability mode in the parent. - sleep(1); + if (verbose) fprintf(stderr, " first child started\n"); + EXPECT_OK(close(proc_pipe[0])); + // Child: wait and then confirm this process is unaffected by capability mode in the parent. + AWAIT_INT_MESSAGE(proc_pipe[1], MSG_PARENT_CHILD_SHOULD_RUN); int fd = open("/dev/null", O_RDWR); EXPECT_OK(fd); close(fd); - exit(0); + // Notify the parent of success/failure. + int rval = (int)testing::Test::HasFailure(); + SEND_INT_MESSAGE(proc_pipe[1], rval); + if (verbose) fprintf(stderr, " first child finished: %d\n", rval); + exit(rval); } EXPECT_OK(cap_enter()); // Enter capability mode. + // At this point the current process has both a child process and a + // child thread that were created before entering capability mode. + // - The child process is unaffected by capability mode. + // - The child thread is affected by capability mode. + SEND_INT_MESSAGE(proc_pipe[0], MSG_PARENT_CHILD_SHOULD_RUN); + // Do an allowed syscall. EXPECT_OK(getpid_()); + // Wait for the first child to exit (should get a zero exit code message). + AWAIT_INT_MESSAGE(proc_pipe[0], 0); + + // The child processes/threads return HasFailure(), so we depend on no prior errors. + ASSERT_FALSE(testing::Test::HasFailure()) + << "Cannot continue test with pre-existing failures."; + // Now that we're in capability mode, if we create a second child process + // it will be affected by capability mode. + if (verbose) fprintf(stderr, " starting second child (in capability mode)\n"); int child = fork(); EXPECT_OK(child); if (child == 0) { + if (verbose) fprintf(stderr, " second child started\n"); + EXPECT_OK(close(proc_pipe[0])); // Child: do an allowed and a disallowed syscall. EXPECT_OK(getpid_()); EXPECT_CAPMODE(open("/dev/null", O_RDWR)); - exit(0); + // Notify the parent of success/failure. + int rval = (int)testing::Test::HasFailure(); + SEND_INT_MESSAGE(proc_pipe[1], rval); + if (verbose) fprintf(stderr, " second child finished: %d\n", rval); + exit(rval); } - // Don't (can't) wait for either child. - + // Now tell the early_started thread that it can run. We expect it to also + // be affected by capability mode since it's per-process not per-thread. + // Note: it is important that we don't allow the thread to run before fork(), + // since that could result in fork() being called while the thread holds one + // of the gtest-internal mutexes, so the child process deadlocks. + SEND_INT_MESSAGE(thread_pipe[0], MSG_PARENT_CHILD_SHOULD_RUN); // Wait for the early-started thread. - EXPECT_OK(pthread_join(early_thread, NULL)); + EXPECT_OK(pthread_join(early_thread, &thread_rval)); + EXPECT_FALSE((bool)(intptr_t)thread_rval) << "thread returned failure"; - // Fire off a new thread. + // Wait for the second child to exit (should get a zero exit code message). + AWAIT_INT_MESSAGE(proc_pipe[0], 0); + + // Fire off a new (second) child thread, which is also affected by capability mode. + ASSERT_FALSE(testing::Test::HasFailure()) + << "Cannot continue test with pre-existing failures."; pthread_t child_thread; - int zero = 0; // seconds - EXPECT_OK(pthread_create(&child_thread, NULL, thread_fn, &zero)); - EXPECT_OK(pthread_join(child_thread, NULL)); + EXPECT_OK(pthread_create(&child_thread, NULL, thread_fn, + (void *)(intptr_t)thread_pipe[1])); + SEND_INT_MESSAGE(thread_pipe[0], MSG_PARENT_CHILD_SHOULD_RUN); + EXPECT_OK(pthread_join(child_thread, &thread_rval)); + EXPECT_FALSE((bool)(intptr_t)thread_rval) << "thread returned failure"; // Fork a subprocess which fires off a new thread. + ASSERT_FALSE(testing::Test::HasFailure()) + << "Cannot continue test with pre-existing failures."; + if (verbose) fprintf(stderr, " starting third child (in capability mode)\n"); child = fork(); EXPECT_OK(child); if (child == 0) { + if (verbose) fprintf(stderr, " third child started\n"); + EXPECT_OK(close(proc_pipe[0])); pthread_t child_thread2; - EXPECT_OK(pthread_create(&child_thread2, NULL, thread_fn, &zero)); - EXPECT_OK(pthread_join(child_thread2, NULL)); - exit(0); + EXPECT_OK(pthread_create(&child_thread2, NULL, thread_fn, + (void *)(intptr_t)thread_pipe[1])); + SEND_INT_MESSAGE(thread_pipe[0], MSG_PARENT_CHILD_SHOULD_RUN); + EXPECT_OK(pthread_join(child_thread2, &thread_rval)); + EXPECT_FALSE((bool)(intptr_t)thread_rval) << "thread returned failure"; + // Notify the parent of success/failure. + int rval = (int)testing::Test::HasFailure(); + SEND_INT_MESSAGE(proc_pipe[1], rval); + if (verbose) fprintf(stderr, " third child finished: %d\n", rval); + exit(rval); } - // Sleep for a bit to allow the subprocess to finish. - sleep(2); + // Wait for the third child to exit (should get a zero exit code message). + AWAIT_INT_MESSAGE(proc_pipe[0], 0); + close(proc_pipe[0]); + close(proc_pipe[1]); + close(thread_pipe[0]); + close(thread_pipe[1]); } -static int had_signal = 0; +static volatile sig_atomic_t had_signal = 0; static void handle_signal(int) { had_signal = 1; } FORK_TEST(Capmode, SelfKill) { diff --git a/contrib/capsicum-test/capsicum-test.cc b/contrib/capsicum-test/capsicum-test.cc index 6adb222ec055..dedad464a4d9 100644 --- a/contrib/capsicum-test/capsicum-test.cc +++ b/contrib/capsicum-test/capsicum-test.cc @@ -1,5 +1,15 @@ #include "capsicum-test.h" +#ifdef __FreeBSD__ +#include <sys/param.h> +#include <sys/proc.h> +#include <sys/queue.h> +#include <sys/socket.h> +#include <sys/sysctl.h> +#include <sys/user.h> +#include <libprocstat.h> +#endif + #include <stdio.h> #include <string.h> #include <signal.h> @@ -48,31 +58,59 @@ char ProcessState(int pid) { return '?'; #endif #ifdef __FreeBSD__ - char buffer[1024]; - snprintf(buffer, sizeof(buffer), "ps -p %d -o state | grep -v STAT", pid); - sig_t original = signal(SIGCHLD, SIG_IGN); - FILE* cmd = popen(buffer, "r"); - usleep(50000); // allow any pending SIGCHLD signals to arrive - signal(SIGCHLD, original); - int result = fgetc(cmd); - fclose(cmd); - // Map FreeBSD codes to Linux codes. - switch (result) { - case EOF: - return '\0'; - case 'D': // disk wait - case 'R': // runnable - case 'S': // sleeping - case 'T': // stopped - case 'Z': // zombie - return result; - case 'W': // idle interrupt thread - return 'S'; - case 'I': // idle - return 'S'; - case 'L': // waiting to acquire lock - default: - return '?'; + // First check if the process exists/we have permission to see it. This + // Avoids warning messages being printed to stderr by libprocstat. + size_t len = 0; + int name[4]; + name[0] = CTL_KERN; + name[1] = KERN_PROC; + name[2] = KERN_PROC_PID; + name[3] = pid; + if (sysctl(name, nitems(name), NULL, &len, NULL, 0) < 0 && errno == ESRCH) { + if (verbose) fprintf(stderr, "Process %d does not exist\n", pid); + return '\0'; // No such process. + } + unsigned int count = 0; + struct procstat *prstat = procstat_open_sysctl(); + EXPECT_NE(NULL, prstat) << "procstat_open_sysctl failed."; + errno = 0; + struct kinfo_proc *p = procstat_getprocs(prstat, KERN_PROC_PID, pid, &count); + if (p == NULL || count == 0) { + if (verbose) fprintf(stderr, "procstat_getprocs failed with %p/%d: %s\n", p, count, strerror(errno)); + procstat_close(prstat); + return '\0'; + } + char result = '\0'; + // See state() in bin/ps/print.c + switch (p->ki_stat) { + case SSTOP: + result = 'T'; + break; + case SSLEEP: + if (p->ki_tdflags & TDF_SINTR) /* interruptable (long) */ + result = 'S'; + else + result = 'D'; + break; + case SRUN: + case SIDL: + result = 'R'; + break; + case SWAIT: + case SLOCK: + // We treat SWAIT/SLOCK as 'S' here (instead of 'W'/'L'). + result = 'S'; + break; + case SZOMB: + result = 'Z'; + break; + default: + result = '?'; + break; } + procstat_freeprocs(prstat, p); + procstat_close(prstat); + if (verbose) fprintf(stderr, "Process %d in state '%c'\n", pid, result); + return result; #endif } diff --git a/contrib/capsicum-test/capsicum-test.h b/contrib/capsicum-test/capsicum-test.h index 808840f4280e..7433814b31c8 100644 --- a/contrib/capsicum-test/capsicum-test.h +++ b/contrib/capsicum-test/capsicum-test.h @@ -140,15 +140,17 @@ const char *TmpFile(const char *pathname); // Expect a syscall to fail with the given error. #define EXPECT_SYSCALL_FAIL(E, C) \ do { \ + SCOPED_TRACE(#C); \ EXPECT_GT(0, C); \ - EXPECT_EQ(E, errno); \ + EXPECT_EQ(E, errno) << "expected '" << strerror(E) \ + << "' but got '" << strerror(errno) << "'"; \ } while (0) // Expect a syscall to fail with anything other than the given error. #define EXPECT_SYSCALL_FAIL_NOT(E, C) \ do { \ EXPECT_GT(0, C); \ - EXPECT_NE(E, errno); \ + EXPECT_NE(E, errno) << strerror(E); \ } while (0) // Expect a void syscall to fail with anything other than the given error. @@ -163,7 +165,8 @@ const char *TmpFile(const char *pathname); // code is OS-specific. #ifdef O_BENEATH #define EXPECT_OPENAT_FAIL_TRAVERSAL(fd, path, flags) \ - do { \ + do { \ + SCOPED_TRACE(GTEST_STRINGIFY_(openat((fd), (path), (flags)))); \ const int result = openat((fd), (path), (flags)); \ if (((flags) & O_BENEATH) == O_BENEATH) { \ EXPECT_SYSCALL_FAIL(E_NO_TRAVERSE_O_BENEATH, result); \ @@ -175,6 +178,7 @@ const char *TmpFile(const char *pathname); #else #define EXPECT_OPENAT_FAIL_TRAVERSAL(fd, path, flags) \ do { \ + SCOPED_TRACE(GTEST_STRINGIFY_(openat((fd), (path), (flags)))); \ const int result = openat((fd), (path), (flags)); \ EXPECT_SYSCALL_FAIL(E_NO_TRAVERSE_CAPABILITY, result); \ if (result >= 0) { close(result); } \ @@ -200,7 +204,8 @@ const char *TmpFile(const char *pathname); int rc = C; \ EXPECT_GT(0, rc); \ EXPECT_TRUE(errno == ECAPMODE || errno == ENOTCAPABLE) \ - << #C << " did not fail with ECAPMODE/ENOTCAPABLE but " << errno; \ + << #C << " did not fail with ECAPMODE/ENOTCAPABLE but " << errno \ + << "(" << strerror(errno) << ")"; \ } while (0) // Ensure that 'rights' are a subset of 'max'. @@ -244,6 +249,29 @@ char ProcessState(int pid); #define EXPECT_PID_ZOMBIE(pid) EXPECT_PID_REACHES_STATES(pid, 'Z', 'Z'); #define EXPECT_PID_GONE(pid) EXPECT_PID_REACHES_STATES(pid, '\0', '\0'); +enum { + // Magic numbers for messages sent by child processes. + MSG_CHILD_STARTED = 1234, + MSG_CHILD_FD_RECEIVED = 4321, + // Magic numbers for messages sent by parent processes. + MSG_PARENT_REQUEST_CHILD_EXIT = 9999, + MSG_PARENT_CLOSED_FD = 10000, + MSG_PARENT_CHILD_SHOULD_RUN = 10001, +}; + +#define SEND_INT_MESSAGE(fd, message) \ + do { \ + int _msg = message; \ + EXPECT_EQ(sizeof(_msg), (size_t)write(fd, &_msg, sizeof(_msg))); \ + } while (0) + +#define AWAIT_INT_MESSAGE(fd, expected) \ + do { \ + int _msg = 0; \ + EXPECT_EQ(sizeof(_msg), (size_t)read(fd, &_msg, sizeof(_msg))); \ + EXPECT_EQ(expected, _msg); \ + } while (0) + // Mark a test that can only be run as root. #define GTEST_SKIP_IF_NOT_ROOT() \ if (getuid() != 0) { GTEST_SKIP() << "requires root"; } diff --git a/contrib/capsicum-test/makefile b/contrib/capsicum-test/makefile index 7b95e1927927..ad697f160e2e 100644 --- a/contrib/capsicum-test/makefile +++ b/contrib/capsicum-test/makefile @@ -8,7 +8,7 @@ CXXFLAGS+=$(ARCHFLAG) -Wall -g $(GTEST_INCS) $(GTEST_FLAGS) --std=c++11 CFLAGS+=$(ARCHFLAG) -Wall -g capsicum-test: $(OBJECTS) libgtest.a $(LOCAL_LIBS) - $(CXX) $(CXXFLAGS) -g -o $@ $(OBJECTS) libgtest.a -lpthread -lrt $(LIBSCTP) $(LIBCAPRIGHTS) + $(CXX) $(CXXFLAGS) -g -o $@ $(OBJECTS) libgtest.a -lpthread -lrt $(LIBSCTP) $(LIBCAPRIGHTS) $(EXTRA_LIBS) # Small statically-linked program for fexecve tests # (needs to be statically linked so that execve()ing it diff --git a/contrib/capsicum-test/openat.cc b/contrib/capsicum-test/openat.cc index 232fb22dd3f7..5447558cde89 100644 --- a/contrib/capsicum-test/openat.cc +++ b/contrib/capsicum-test/openat.cc @@ -11,6 +11,7 @@ // Check an open call works and close the resulting fd. #define EXPECT_OPEN_OK(f) do { \ + SCOPED_TRACE(#f); \ int _fd = f; \ EXPECT_OK(_fd); \ close(_fd); \ diff --git a/contrib/capsicum-test/procdesc.cc b/contrib/capsicum-test/procdesc.cc index 11274ce9e866..105546cabfb2 100644 --- a/contrib/capsicum-test/procdesc.cc +++ b/contrib/capsicum-test/procdesc.cc @@ -27,10 +27,6 @@ #define __WALL 0 #endif -// TODO(drysdale): it would be nice to use proper synchronization between -// processes, rather than synchronization-via-sleep; faster too. - - //------------------------------------------------ // Utilities for the tests. @@ -73,7 +69,10 @@ static void print_stat(FILE *f, const struct stat *stat) { (long)stat->st_atime, (long)stat->st_mtime, (long)stat->st_ctime); } -static std::map<int,bool> had_signal; +static volatile sig_atomic_t had_signal[NSIG]; +void clear_had_signals() { + memset(const_cast<sig_atomic_t *>(had_signal), 0, sizeof(had_signal)); +} static void handle_signal(int x) { had_signal[x] = true; } @@ -109,7 +108,9 @@ void CheckChildFinished(pid_t pid, bool signaled=false) { TEST(Pdfork, Simple) { int pd = -1; + int pipefds[2]; pid_t parent = getpid_(); + EXPECT_OK(pipe(pipefds)); int pid = pdfork(&pd, 0); EXPECT_OK(pid); if (pid == 0) { @@ -117,19 +118,29 @@ TEST(Pdfork, Simple) { EXPECT_EQ(-1, pd); EXPECT_NE(parent, getpid_()); EXPECT_EQ(parent, getppid()); - sleep(1); - exit(0); + close(pipefds[0]); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); + if (verbose) fprintf(stderr, "Child waiting for exit message\n"); + // Terminate once the parent has completed the checks + AWAIT_INT_MESSAGE(pipefds[1], MSG_PARENT_REQUEST_CHILD_EXIT); + exit(testing::Test::HasFailure()); } - usleep(100); // ensure the child has a chance to run + close(pipefds[1]); + // Ensure the child has started. + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); + EXPECT_NE(-1, pd); EXPECT_PID_ALIVE(pid); int pid_got; EXPECT_OK(pdgetpid(pd, &pid_got)); EXPECT_EQ(pid, pid_got); - // Wait long enough for the child to exit(). - sleep(2); + // Tell the child to exit and wait until it is a zombie. + SEND_INT_MESSAGE(pipefds[0], MSG_PARENT_REQUEST_CHILD_EXIT); + // EXPECT_PID_ZOMBIE waits for up to ~500ms, that should be enough time for + // the child to exit successfully. EXPECT_PID_ZOMBIE(pid); + close(pipefds[0]); // Wait for the the child. int status; @@ -223,7 +234,10 @@ TEST(Pdfork, NonProcessDescriptor) { close(fd); } -static void *SubThreadMain(void *) { +static void *SubThreadMain(void *arg) { + // Notify the main thread that we have started + if (verbose) fprintf(stderr, " subthread started: pipe=%p\n", arg); + SEND_INT_MESSAGE((int)(intptr_t)arg, MSG_CHILD_STARTED); while (true) { if (verbose) fprintf(stderr, " subthread: \"I aten't dead\"\n"); usleep(100000); @@ -233,11 +247,28 @@ static void *SubThreadMain(void *) { static void *ThreadMain(void *) { int pd; + int pipefds[2]; + EXPECT_EQ(0, pipe(pipefds)); pid_t child = pdfork(&pd, 0); if (child == 0) { - // Child: start a subthread then loop + close(pipefds[0]); + // Child: start a subthread then loop. pthread_t child_subthread; - EXPECT_OK(pthread_create(&child_subthread, NULL, SubThreadMain, NULL)); + // Wait for the subthread startup using another pipe. + int thread_pipefds[2]; + EXPECT_EQ(0, pipe(thread_pipefds)); + EXPECT_OK(pthread_create(&child_subthread, NULL, SubThreadMain, + (void *)(intptr_t)thread_pipefds[0])); + if (verbose) { + fprintf(stderr, " pdforked process %d: waiting for subthread.\n", + getpid()); + } + AWAIT_INT_MESSAGE(thread_pipefds[1], MSG_CHILD_STARTED); + close(thread_pipefds[0]); + close(thread_pipefds[1]); + // Child: Notify parent that all threads have started + if (verbose) fprintf(stderr, " pdforked process %d: subthread started\n", getpid()); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); while (true) { if (verbose) fprintf(stderr, " pdforked process %d: \"I aten't dead\"\n", getpid()); usleep(100000); @@ -245,7 +276,9 @@ static void *ThreadMain(void *) { exit(0); } if (verbose) fprintf(stderr, " thread generated pd %d\n", pd); - sleep(2); + close(pipefds[1]); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); + if (verbose) fprintf(stderr, "[%d] got child startup message\n", getpid_()); // Pass the process descriptor back to the main thread. return reinterpret_cast<void *>(pd); @@ -278,7 +311,7 @@ TEST(Pdfork, FromThread) { class PipePdforkBase : public ::testing::Test { public: PipePdforkBase(int pdfork_flags) : pd_(-1), pid_(-1) { - had_signal.clear(); + clear_had_signals(); int pipes[2]; EXPECT_OK(pipe(pipes)); pipe_ = pipes[1]; @@ -356,24 +389,34 @@ TEST_F(PipePdfork, Poll) { // Can multiple processes poll on the same descriptor? TEST_F(PipePdfork, PollMultiple) { + int pipefds[2]; + EXPECT_EQ(0, pipe(pipefds)); int child = fork(); EXPECT_OK(child); if (child == 0) { - // Child: wait to give time for setup, then write to the pipe (which will - // induce exit of the pdfork()ed process) and exit. - sleep(1); + close(pipefds[0]); + // Child: wait for parent to acknowledge startup + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); + // Child: wait for two messages from the parent and the forked process + // before telling the other process to terminate. + if (verbose) fprintf(stderr, "[%d] waiting for read 1\n", getpid_()); + AWAIT_INT_MESSAGE(pipefds[1], MSG_PARENT_REQUEST_CHILD_EXIT); + if (verbose) fprintf(stderr, "[%d] waiting for read 2\n", getpid_()); + AWAIT_INT_MESSAGE(pipefds[1], MSG_PARENT_REQUEST_CHILD_EXIT); TerminateChild(); - exit(0); + if (verbose) fprintf(stderr, "[%d] about to exit\n", getpid_()); + exit(testing::Test::HasFailure()); } - usleep(100); // ensure the child has a chance to run - + close(pipefds[1]); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); + if (verbose) fprintf(stderr, "[%d] got child startup message\n", getpid_()); // Fork again int doppel = fork(); EXPECT_OK(doppel); // We now have: // pid A: main process, here // |--pid B: pdfork()ed process, blocked on read() - // |--pid C: fork()ed process, in sleep(1) above + // |--pid C: fork()ed process, in read() above // +--pid D: doppel process, here // Both A and D execute the following code. @@ -384,12 +427,18 @@ TEST_F(PipePdfork, PollMultiple) { fdp.revents = 0; EXPECT_EQ(0, poll(&fdp, 1, 0)); + // Both A and D ask C to exit, allowing it to do so. + if (verbose) fprintf(stderr, "[%d] telling child to exit\n", getpid_()); + SEND_INT_MESSAGE(pipefds[0], MSG_PARENT_REQUEST_CHILD_EXIT); + close(pipefds[0]); + // Now, wait (indefinitely) for activity on the process descriptor. // We expect: - // - pid C will finish its sleep, write to the pipe and exit + // - pid C will finish its two read() calls, write to the pipe and exit. // - pid B will unblock from read(), and exit // - this will generate an event on the process descriptor... // - ...in both process A and process D. + if (verbose) fprintf(stderr, "[%d] waiting for child to exit\n", getpid_()); EXPECT_EQ(1, poll(&fdp, 1, 2000)); EXPECT_TRUE(fdp.revents & POLLHUP); @@ -522,6 +571,7 @@ TEST_F(PipePdfork, CloseLast) { FORK_TEST(Pdfork, OtherUserIfRoot) { GTEST_SKIP_IF_NOT_ROOT(); int pd; + int status; pid_t pid = pdfork(&pd, 0); EXPECT_OK(pid); if (pid == 0) { @@ -542,15 +592,29 @@ FORK_TEST(Pdfork, OtherUserIfRoot) { EXPECT_EQ(EPERM, errno); EXPECT_PID_ALIVE(pid); - // Succeed with pdkill though. + // Ideally, we should be able to send signals via a process descriptor even + // if it's owned by another user, but this is not implementated on FreeBSD. +#ifdef __FreeBSD__ + // On FreeBSD, pdkill() still performs all the same checks that kill() does + // and therefore cannot be used to send a signal to a process with another + // UID unless we are root. + EXPECT_SYSCALL_FAIL(EBADF, pdkill(pid, SIGKILL)); + EXPECT_PID_ALIVE(pid); + // However, the process will be killed when we close the process descriptor. + EXPECT_OK(close(pd)); + EXPECT_PID_GONE(pid); + // Can't pdwait4() after close() since close() reparents the child to a reaper (init) + EXPECT_SYSCALL_FAIL(EBADF, pdwait4_(pd, &status, WNOHANG, NULL)); +#else + // Sending a signal with pdkill() should be permitted though. EXPECT_OK(pdkill(pd, SIGKILL)); EXPECT_PID_ZOMBIE(pid); - int status; int rc = pdwait4_(pd, &status, WNOHANG, NULL); EXPECT_OK(rc); EXPECT_EQ(pid, rc); EXPECT_TRUE(WIFSIGNALED(status)); +#endif } TEST_F(PipePdfork, WaitPidThenPd) { @@ -624,18 +688,27 @@ TEST_F(PipePdforkDaemon, Pdkill) { TEST(Pdfork, PdkillOtherSignal) { int pd = -1; + int pipefds[2]; + EXPECT_EQ(0, pipe(pipefds)); int pid = pdfork(&pd, 0); EXPECT_OK(pid); if (pid == 0) { - // Child: watch for SIGUSR1 forever. - had_signal.clear(); + // Child: tell the parent that we have started before entering the loop, + // and importantly only do so once we have registered the SIGUSR1 handler. + close(pipefds[0]); + clear_had_signals(); signal(SIGUSR1, handle_signal); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); + // Child: watch for SIGUSR1 forever. while (!had_signal[SIGUSR1]) { usleep(100000); } exit(123); } - sleep(1); + // Wait for child to start + close(pipefds[1]); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); + close(pipefds[0]); // Send an invalid signal. EXPECT_EQ(-1, pdkill(pd, 0xFFFF)); @@ -651,14 +724,15 @@ TEST(Pdfork, PdkillOtherSignal) { int rc = waitpid(pid, &status, __WALL); EXPECT_OK(rc); EXPECT_EQ(pid, rc); - EXPECT_TRUE(WIFEXITED(status)) << "0x" << std::hex << rc; + EXPECT_TRUE(WIFEXITED(status)) << "status: 0x" << std::hex << status; EXPECT_EQ(123, WEXITSTATUS(status)); } pid_t PdforkParentDeath(int pdfork_flags) { // Set up: // pid A: main process, here - // +--pid B: fork()ed process, sleep(4)s then exits + // +--pid B: fork()ed process, starts a child process with pdfork() then + // waits for parent to send a shutdown message. // +--pid C: pdfork()ed process, looping forever int sock_fds[2]; EXPECT_OK(socketpair(AF_UNIX, SOCK_STREAM, 0, sock_fds)); @@ -668,27 +742,45 @@ pid_t PdforkParentDeath(int pdfork_flags) { if (child == 0) { int pd; if (verbose) fprintf(stderr, " [%d] child about to pdfork()...\n", getpid_()); + int pipefds[2]; // for startup notification + EXPECT_OK(pipe(pipefds)); pid_t grandchild = pdfork(&pd, pdfork_flags); if (grandchild == 0) { + close(pipefds[0]); + pid_t grandchildPid = getpid_(); + EXPECT_EQ(sizeof(grandchildPid), (size_t)write(pipefds[1], &grandchildPid, sizeof(grandchildPid))); while (true) { - if (verbose) fprintf(stderr, " [%d] grandchild: \"I aten't dead\"\n", getpid_()); + if (verbose) fprintf(stderr, " [%d] grandchild: \"I aten't dead\"\n", grandchildPid); sleep(1); } } + close(pipefds[1]); if (verbose) fprintf(stderr, " [%d] pdfork()ed grandchild %d, sending ID to parent\n", getpid_(), grandchild); - // send grandchild pid to parent - write(sock_fds[1], &grandchild, sizeof(grandchild)); - sleep(4); + // Wait for grandchild to start. + pid_t grandchild2; + EXPECT_EQ(sizeof(grandchild2), (size_t)read(pipefds[0], &grandchild2, sizeof(grandchild2))); + EXPECT_EQ(grandchild, grandchild2) << "received invalid grandchild pid"; + if (verbose) fprintf(stderr, " [%d] grandchild %d has started successfully\n", getpid_(), grandchild); + close(pipefds[0]); + + // Send grandchild pid to parent. + EXPECT_EQ(sizeof(grandchild), (size_t)write(sock_fds[1], &grandchild, sizeof(grandchild))); + if (verbose) fprintf(stderr, " [%d] sent grandchild pid %d to parent\n", getpid_(), grandchild); + // Wait for parent to acknowledge the message. + AWAIT_INT_MESSAGE(sock_fds[1], MSG_PARENT_REQUEST_CHILD_EXIT); + if (verbose) fprintf(stderr, " [%d] parent acknowledged grandchild pid %d\n", getpid_(), grandchild); if (verbose) fprintf(stderr, " [%d] child terminating\n", getpid_()); - exit(0); + exit(testing::Test::HasFailure()); } if (verbose) fprintf(stderr, "[%d] fork()ed child is %d\n", getpid_(), child); pid_t grandchild; read(sock_fds[0], &grandchild, sizeof(grandchild)); - if (verbose) fprintf(stderr, "[%d] receive grandchild id %d\n", getpid_(), grandchild); + if (verbose) fprintf(stderr, "[%d] received grandchild id %d\n", getpid_(), grandchild); EXPECT_PID_ALIVE(child); EXPECT_PID_ALIVE(grandchild); - sleep(6); + // Tell child to exit. + if (verbose) fprintf(stderr, "[%d] telling child %d to exit\n", getpid_(), child); + SEND_INT_MESSAGE(sock_fds[0], MSG_PARENT_REQUEST_CHILD_EXIT); // Child dies, closing its process descriptor for the grandchild. EXPECT_PID_DEAD(child); CheckChildFinished(child); @@ -713,7 +805,7 @@ TEST(Pdfork, BagpussDaemon) { // The exit of a pdfork()ed process should not generate SIGCHLD. TEST_F(PipePdfork, NoSigchld) { - had_signal.clear(); + clear_had_signals(); sighandler_t original = signal(SIGCHLD, handle_signal); TerminateChild(); int rc = 0; @@ -728,7 +820,7 @@ TEST_F(PipePdfork, NoSigchld) { // all been closed should generate SIGCHLD. The child process needs // PD_DAEMON to survive the closure of the process descriptors. TEST_F(PipePdforkDaemon, NoPDSigchld) { - had_signal.clear(); + clear_had_signals(); sighandler_t original = signal(SIGCHLD, handle_signal); EXPECT_OK(close(pd_)); @@ -766,10 +858,8 @@ TEST_F(PipePdfork, ModeBits) { #endif TEST_F(PipePdfork, WildcardWait) { - // TODO(FreeBSD): make wildcard wait ignore pdfork()ed children - // https://bugs.freebsd.org/201054 TerminateChild(); - sleep(1); // Ensure child is truly dead. + EXPECT_PID_ZOMBIE(pid_); // Ensure child is truly dead. // Wildcard waitpid(-1) should not see the pdfork()ed child because // there is still a process descriptor for it. @@ -782,21 +872,30 @@ TEST_F(PipePdfork, WildcardWait) { } FORK_TEST(Pdfork, Pdkill) { - had_signal.clear(); + clear_had_signals(); int pd; + int pipefds[2]; + EXPECT_OK(pipe(pipefds)); pid_t pid = pdfork(&pd, 0); EXPECT_OK(pid); if (pid == 0) { - // Child: set a SIGINT handler and sleep. - had_signal.clear(); + // Child: set a SIGINT handler, notify the parent and sleep. + close(pipefds[0]); + clear_had_signals(); signal(SIGINT, handle_signal); + if (verbose) fprintf(stderr, "[%d] child started\n", getpid_()); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); if (verbose) fprintf(stderr, "[%d] child about to sleep(10)\n", getpid_()); - int left = sleep(10); - if (verbose) fprintf(stderr, "[%d] child slept, %d sec left, had[SIGINT]=%d\n", - getpid_(), left, had_signal[SIGINT]); - // Expect this sleep to be interrupted by the signal (and so left > 0). - exit(left == 0); + // Note: we could receive the SIGINT just before sleep(), so we use a loop + // with a short delay instead of one long sleep(). + for (int i = 0; i < 50 && !had_signal[SIGINT]; i++) { + usleep(100000); + } + if (verbose) fprintf(stderr, "[%d] child slept, had[SIGINT]=%d\n", + getpid_(), (int)had_signal[SIGINT]); + // Return non-zero if we didn't see SIGINT. + exit(had_signal[SIGINT] ? 0 : 99); } // Parent: get child's PID. @@ -804,9 +903,12 @@ FORK_TEST(Pdfork, Pdkill) { EXPECT_OK(pdgetpid(pd, &pd_pid)); EXPECT_EQ(pid, pd_pid); - // Interrupt the child after a second. - sleep(1); + // Interrupt the child once it's registered the SIGINT handler. + close(pipefds[1]); + if (verbose) fprintf(stderr, "[%d] waiting for child\n", getpid_()); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); EXPECT_OK(pdkill(pd, SIGINT)); + if (verbose) fprintf(stderr, "[%d] sent SIGINT\n", getpid_()); // Make sure the child finished properly (caught signal then exited). CheckChildFinished(pid); @@ -814,19 +916,28 @@ FORK_TEST(Pdfork, Pdkill) { FORK_TEST(Pdfork, PdkillSignal) { int pd; + int pipefds[2]; + EXPECT_OK(pipe(pipefds)); pid_t pid = pdfork(&pd, 0); EXPECT_OK(pid); if (pid == 0) { - // Child: sleep. No SIGINT handler. - if (verbose) fprintf(stderr, "[%d] child about to sleep(10)\n", getpid_()); - int left = sleep(10); - if (verbose) fprintf(stderr, "[%d] child slept, %d sec left\n", getpid_(), left); + close(pipefds[0]); + if (verbose) fprintf(stderr, "[%d] child started\n", getpid_()); + SEND_INT_MESSAGE(pipefds[1], MSG_CHILD_STARTED); + // Child: wait for shutdown message. No SIGINT handler. The message should + // never be received, since SIGINT should terminate the process. + if (verbose) fprintf(stderr, "[%d] child about to read()\n", getpid_()); + AWAIT_INT_MESSAGE(pipefds[1], MSG_PARENT_REQUEST_CHILD_EXIT); + fprintf(stderr, "[%d] child read() returned unexpectedly\n", getpid_()); exit(99); } - + // Wait for child to start before signalling. + if (verbose) fprintf(stderr, "[%d] waiting for child\n", getpid_()); + close(pipefds[1]); + AWAIT_INT_MESSAGE(pipefds[0], MSG_CHILD_STARTED); // Kill the child (as it doesn't handle SIGINT). - sleep(1); + if (verbose) fprintf(stderr, "[%d] sending SIGINT\n", getpid_()); EXPECT_OK(pdkill(pd, SIGINT)); // Make sure the child finished properly (terminated by signal). @@ -922,7 +1033,7 @@ TEST_F(PipePdfork, PassProcessDescriptor) { if (child2 == 0) { // Child: close our copy of the original process descriptor. *** 121 LINES SKIPPED ***
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103172223.12HMN5Yb045968>