the perils of pause(2) [c, posix, coccinelle]

I recently had a bug in a simple program that has a form I've seen a lot in the last few years: loops and signal handling without masking. The worst thing about these kinds of bugs is that they don't rear their heads immediately – they fall into the class of "huh, it's blocked in a syscall and I'm sure it should have woken up" bugs. Let's look at the problem and then how to lint it.

1. A common mistake

I had some tooling for a test suite that would wait for a specified signal, and then print the name of that signal on stdout. I did this by setting up a signal handler, and then calling pause(), which suspends the program until a signal is delivered (i.e., it always returns EINTR). The program indicates to its cooperating programs that it's ready for the signal by printing "ok".

static sig_atomic_t got;
static void h(int n) { got = n; }
//...
    if (sigaction(sig, &(struct sigaction){.sa_handler=h}, NULL))
        abort();
    write(1, "ok\n", 3);
    do { pause(); } while (sig != got);

Every so often, this program will just hang and the test would time out. Worse yet, it's rare enough that I didn't really notice it when I wrote the code.1 (Why loop when we only expect one signal? There are other signals that will interrupt pause unless you've gone out of your way to ignore them all; for example, SIGTSTP.)

One possible problematic execution is this:

  • we print ok;
  • before we get to pause, the other program sends the signal;
  • now sig == got, but we pause anyway, and wait for another signal that will never come.

Another common execution with this pattern is this:

  • we pause, and get interrupted by some other signal;
  • we test got against our desired sig and see it hasn't triggered;
  • now our desired signal is delivered, and sig == got, but we're already past the test;
  • we pause again, and have to wait arbitrarily long (till some other signal wakes us up).

This also happens often in loops with poll or select:

for (;;) {
    // A) either there's fd activity, or we get EINTR
    int n_active = poll(fds, n_fds, INFTIM);
    // [...] handle fds
    // B) check variables set by signal handler
    ...
}

We expect poll to get interrupted if there's a signal, however the signal may arrive after the test at B but before we get back to A.

The solution in all these cases are signal masks, and calls that manipulate them atomically. When a signal arrives while masked by the process, it remains pending until the process unmasks it.

2. Masking versus disposition

Something that's always confusing about this is that masking a signal does not affect its disposition. "Signal disposition" is the action associated with a signal, that is, what should happen when the signal is delivered to the process: either a handler is called, the signal is ignored, or a default action takes place.

Knowing that, you might set a mask for some signal whose disposition is SIG_DFL, and see that it works fine, and then be confused when this doesn't work for signals whose disposition is SIG_IGN. POSIX says:

If the action associated with a blocked signal is anything other than to ignore the signal, and if that signal is generated for the thread, the signal shall remain pending until it is unblocked, it is accepted when it is selected and returned by a call to the sigwait() function, or the action associated with it is set to ignore the signal. — POSIX.1-2017 System Interfaces 2.4.1 Signal generation and delivery

I noticed that OpenBSD has the slightly strange behavior of treating signals whose default disposition is to stop or continue the program as if they have the ignored disposition, so these signals need an explicit handler. This is probably a bug? Although it means we could have avoided looping in our initial example, so one could argue it's the better behavior.

Also, signal disposition is per-process, while signal masks are per-thread – but let's not get into that mess here. (An even bigger mess is then what happens on exec() – dispositions are reset, but masks are inherited, as well as pending signals!)

3. Alternatives that atomically unmask

In the case of pause(2), we can use sigsuspend(2), or a few other similar functions. If we need the signal handler, our code might look like this:

if (sigaction(sig, &(struct sigaction){.sa_handler=h}, NULL))
    abort();
sigset_t set, prev;
sigemptyset(&set);
sigaddset(&set, sig);
sigprocmask(SIG_BLOCK, &set, &prev);
write(1, "ok\n", 3);
do { sigsuspend(&prev); } while (sig != got);

but in this case, we are just waiting for this signal, and don't need to take other action when it arrives, so sigwait(2) suffices:

sigset_t set, prev;
sigemptyset(&set);
sigaddset(&set, sig);
sigprocmask(SIG_BLOCK, &set, &prev);
write(1, "ok\n", 3);
int got;
do { sigwait(&set, &got); } while (sig != got);

We might use sigwaitinfo or sigtimedwait if we need more details than just the signal number. If we were certain no other signal could arrive, we could avoid the loop entirely, but it's nice to protect against cases you might not otherwise consider, like testing the program interactively and hitting ^Z (sending SIGTSTP).

(Note that OpenBSD also lacks sigwaitinfo and sigtimedwait presently.)

For poll(2) and select(2), unmasking variants ppoll(2) and pselect(2) exist for this reason. (Linux also has signalfd(2), which more naturally integrates with polling loops, but note it only reads pending signals, so you still need to mask with sigprocmask, and now you have to deal with reading siginfo out of a buffer. Oh, and what you actually get out of the fd depends on which process is reading…) There's also the classic self-pipe trick.

4. Linting

This makes me wonder about some kind of review-level lints that only apply to new code being added. Ideally we'd flag any code which accesses variables assigned from signal handlers and calls one of these functions, in a loop.

Here's my attempt at partially doing this with coccinelle:

@@
sig_atomic_t signal_handler_variable;
@@
*   signal_handler_variable
    ...
*   \(pause\|poll\|select\)(...)

@@
sig_atomic_t signal_handler_variable;
@@
*   \(pause\|poll\|select\)(...)
    ...
*   signal_handler_variable

This will match any function that contains access to a sig_atomic_t and a call to pause, poll, or select. If you save that as lint-pause.cocci, you can check code with

spatch --very-quiet --sp-file lint-pause.cocci path/to/c/files

Note that I am just using * to print out match cases for brevity, but you can add Python scripts to coccinelle rules for much prettier/more elaborate reporting.

It's possible to do much more careful matching, like ensuring the poll calls happen in a loop, or only matching polls with no timeout, but this simple form is sufficient to catch interesting cases to examine later. I also discovered while writing a more elaborate form that do {} while matching was only merged last year and distributions tend to carry older version of spatch.

Note that it only catches use of sig_atomic_t; while testing this, I found some old code that just doesn't even use volatile; at some point I may write a more elaborate script that flags all globals set from any function passed to sigaction, but as a review reminder, this simpler form suffices for my needs.

5. Conclusion

I notice that Kerrisk's LPI talks about pause in section 20.14, but doesn't note its perils, except to indicate that other ways will be investigated in section 22.9. There, Kerrisk introduces sigsuspend and talks about exactly our problem with pause.

glibc's info page talks about this extensively, so it's unfortunate that, for example, Linux's man page for pause(2) contains no such details.

Running the aforementioned coccinelle script across an arbitrary corpus of packages I have on hand turns up a number of likely instances of this bug, so this is still an issue worth keeping in mind.

Footnotes:

1

Note that this program was designed to only handle a single signal. If that handler was registered for multiple signals, you could have races around what value got gets. It used to be you could have got be a mask – e.g. got |= 1<<n; – except these days SIGRTMAX and _SIG_MAXSIG can be way higher than whatever the width of sig_atomic_t is, so I guess people end up doing an array of sig_atomic_t which is maybe 64 times less dense than you'd like. :-/

JS