Page Menu
Home
FreeBSD
Search
Configure Global Search
Log In
Files
F143053340
D51024.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Flag For Later
Award Token
Size
4 KB
Referenced Files
None
Subscribers
None
D51024.diff
View Options
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
Details
Attached
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)
Attached To
Mode
D51024: lockf: switch to a SIGCHLD model for reaping child
Attached
Detach File
Event Timeline
Log In to Comment