diff --git a/usr.bin/lockf/lockf.c b/usr.bin/lockf/lockf.c --- a/usr.bin/lockf/lockf.c +++ b/usr.bin/lockf/lockf.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -50,14 +51,19 @@ static int acquire_lock(union lock_subject *subj, int flags, int silent); static void cleanup(void); static void killed(int sig); +static void sigchld(int sig); static void timeout(int sig); static void usage(void) __dead2; static void wait_for_lock(const char *name); static const char *lockname; +_Static_assert(sizeof(sig_atomic_t) >= sizeof(pid_t), + "PIDs cannot be managed safely from a signal handler on this platform."); +static sig_atomic_t child = -1; static int lockfd = -1; static int keep; static int fdlock; +static int status; static volatile sig_atomic_t timed_out; /* @@ -91,10 +97,14 @@ int main(int argc, char **argv) { - int ch, flags, silent, status, writepid; + struct sigaction sa_chld = { + .sa_handler = sigchld, + .sa_flags = SA_NOCLDSTOP, + }; + sigset_t mask; long long waitsec; - pid_t child; union lock_subject subj; + int ch, flags, silent, writepid; silent = keep = writepid = 0; flags = O_CREAT | O_RDONLY; @@ -250,16 +260,33 @@ signal(SIGINT, SIG_IGN); signal(SIGQUIT, SIG_IGN); signal(SIGTERM, killed); + + /* + * Block SIGTERM while SIGCHLD is being processed, so that we can safely + * waitpid(2) for the child without a concurrent termination observing + * an invalid pid (i.e., waited-on). + */ + (void)sigprocmask(SIG_BLOCK, NULL, &mask); + memcpy(&sa_chld.sa_mask, &mask, sizeof(mask)); + sigaddset(&sa_chld.sa_mask, SIGTERM); + sigaction(SIGCHLD, &sa_chld, NULL); + fclose(stdin); fclose(stdout); fclose(stderr); /* Write out the pid before we sleep on it. */ if (writepid) - (void)dprintf(lockfd, "%d\n", child); + (void)dprintf(lockfd, "%d\n", (int)child); + + sigdelset(&mask, SIGCHLD); + sigdelset(&mask, SIGTERM); + while (child >= 0) { + (void)sigsuspend(&mask); + /* child */ + atomic_signal_fence(memory_order_acquire); + } - if (waitpid(child, &status, 0) == -1) - exit(EX_OSERR); return (WIFEXITED(status) ? WEXITSTATUS(status) : EX_SOFTWARE); } @@ -323,6 +350,26 @@ _Exit(EX_OSERR); } +/* + * Signal handler for SIGCHLD. Simply waits for the child and ensures that we + * don't end up in a sticky situation if we receive a SIGTERM around the same + * time. + */ +static void +sigchld(int sig __unused) +{ + int ostatus; + + while (waitpid(child, &ostatus, 0) != child) { + if (errno != EINTR) + _exit(EX_OSERR); + } + + status = ostatus; + child = -1; + atomic_signal_fence(memory_order_release); +} + /* * Signal handler for SIGALRM. */ diff --git a/usr.bin/lockf/tests/lockf_test.sh b/usr.bin/lockf/tests/lockf_test.sh --- a/usr.bin/lockf/tests/lockf_test.sh +++ b/usr.bin/lockf/tests/lockf_test.sh @@ -62,6 +62,13 @@ atf_check test ! -e "testlock" } +atf_test_case bubble_error +bubble_error_body() +{ + # Ensure that lockf bubbles up the error as expected. + atf_check -s exit:9 lockf testlock sh -c 'exit 9' +} + atf_test_case fdlock fdlock_body() { @@ -233,6 +240,7 @@ { atf_add_test_case badargs atf_add_test_case basic + atf_add_test_case bubble_error atf_add_test_case fdlock atf_add_test_case keep atf_add_test_case needfile