Page MenuHomeFreeBSD

D51024.diff
No OneTemporary

D51024.diff

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 <fcntl.h>
#include <limits.h>
#include <signal.h>
+#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -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,
+ }, sa_prev;
+ sigset_t mask, omask;
long long waitsec;
- pid_t child;
union lock_subject subj;
+ int ch, flags, silent, writepid;
silent = keep = writepid = 0;
flags = O_CREAT | O_RDONLY;
@@ -238,9 +248,30 @@
if (atexit(cleanup) == -1)
err(EX_OSERR, "atexit failed");
+
+ /*
+ * 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). If our setup between here and the
+ * sigsuspend loop gets any more complicated, we should rewrite it to
+ * just use a pipe to signal the child onto execvp().
+ *
+ * We're blocking SIGCHLD and SIGTERM here so that we don't do any
+ * cleanup before we're ready to (after the pid is written out).
+ */
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGCHLD);
+ sigaddset(&mask, SIGTERM);
+ (void)sigprocmask(SIG_BLOCK, &mask, &omask);
+
+ memcpy(&sa_chld.sa_mask, &omask, sizeof(omask));
+ sigaddset(&sa_chld.sa_mask, SIGTERM);
+ (void)sigaction(SIGCHLD, &sa_chld, &sa_prev);
+
if ((child = fork()) == -1)
err(EX_OSERR, "cannot fork");
if (child == 0) { /* The child process. */
+ (void)sigprocmask(SIG_SETMASK, &omask, NULL);
close(lockfd);
execvp(argv[0], argv);
warn("%s", argv[0]);
@@ -250,16 +281,24 @@
signal(SIGINT, SIG_IGN);
signal(SIGQUIT, SIG_IGN);
signal(SIGTERM, killed);
+
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);
+
+ /* Just in case they were blocked on entry. */
+ sigdelset(&omask, SIGCHLD);
+ sigdelset(&omask, SIGTERM);
+ while (child >= 0) {
+ (void)sigsuspend(&omask);
+ /* 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 +362,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

File Metadata

Mime Type
text/plain
Expires
Mon, Jan 26, 12:45 PM (14 h, 51 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
28008065
Default Alt Text
D51024.diff (4 KB)

Event Timeline