CWE-479: Signal Handler Use of a Non-reentrant Function - C

Overview

A signal handler runs asynchronously and can interrupt the main program - or another handler - mid-operation. If the handler then calls a function that isn't reentrant (one that uses static/global state, internal locks, or its own buffers), it can observe or corrupt state the interrupted code was in the middle of using. The result ranges from garbled output to deadlock to heap corruption, and it's timing-dependent, so it often doesn't show up in normal testing.

Primary Defence: Restrict every signal handler to functions POSIX documents as async-signal-safe (write(), _exit(), sig_atomic_t assignment, and a short list of others), and defer everything else - including malloc, printf, and mutex locks - to the normal execution context.

Common Vulnerable Patterns

Non-Reentrant Functions in a Handler

void handler(int sig) {
    // VULNERABLE - malloc is not reentrant
    char *msg = malloc(100);

    // VULNERABLE - printf is not async-signal-safe
    printf("Signal %d\n", sig);

    // VULNERABLE - syslog looks like a system call and is not one
    syslog(LOG_WARNING, "signal %d", sig);

    // VULNERABLE - can deadlock
    pthread_mutex_lock(&lock);

    // VULNERABLE - exit() is not async-signal-safe
    exit(1);  // use _exit(1) instead
}

Why this is vulnerable: Each of these functions maintains internal state - the heap allocator's free lists, stdio's internal buffers, the mutex's own locking - that assumes it won't be re-entered while a call is in progress. If the signal arrives while the main thread is already inside one of these functions, the handler's call reuses state that's mid-update, corrupting it. exit() additionally runs the full atexit/cleanup sequence, which itself may call non-reentrant code - _exit() skips all of that and terminates immediately.

syslog() is on the list because it is MITRE's own demonstrative example for this CWE and because it is the one that does not look like a mistake: it reads as a thin wrapper over a system call, and it is not - it formats into a buffer, holds a static connection to /dev/log, and allocates. What decides safety is not how low-level a function looks but whether the platform lists it, and the list is short enough to read: signal-safety(7) for POSIX, and the equivalent section of your platform's documentation elsewhere.

Deadlock via Reacquired Lock

// VULNERABLE - Deadlock via Reacquired Lock
// Main thread
pthread_mutex_lock(&lock);   // acquired
// ... SIGNAL INTERRUPTS HERE ...

// Signal handler, running on the same thread
pthread_mutex_lock(&lock);   // DEADLOCK - already held, and this thread
                              // is now the one that would need to release it

Why this is vulnerable: The signal handler runs on the same thread that was interrupted. If that thread already holds the lock when the signal arrives, the handler's attempt to acquire the same lock blocks forever - the only thread that could release it is suspended waiting for the handler to return, and the handler is waiting for the lock. Nothing breaks this cycle without external intervention.

Secure Patterns

Minimal Handler, Deferred Work

#include <signal.h>
#include <stdio.h>
#include <unistd.h>

volatile sig_atomic_t got_signal = 0;

static void handler(int sig) {
    got_signal = sig;  // only safe, non-reentrant-function-free work
}

int main(void) {
    struct sigaction sa;
    sa.sa_handler = handler;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_RESTART;
    sigaction(SIGINT, &sa, NULL);

    while (1) {
        if (got_signal) {
            printf("Received signal %d\n", (int)got_signal);  // safe here - normal context
            cleanup();
            break;
        }
        do_work();
    }
    return 0;
}

Why this works: The handler touches only a volatile sig_atomic_t, which is safe to write from a handler regardless of what the interrupted code was doing - it is one of the two kinds of object a handler may touch at all, the other being a lock-free atomic. Every non-reentrant call - printf, cleanup(), anything else - happens in the main loop, in normal execution context, where no function is being re-entered and none of the restrictions apply.

The registration is sigaction() rather than signal() deliberately. signal()'s semantics for establishing a handler are implementation-defined - the man page's summary is that "the only portable use of signal() is to set a signal's disposition to SIG_DFL or SIG_IGN" - and the two behaviours it is allowed to choose between break differently. Under the original System V semantics the disposition resets to SIG_DFL on delivery and nothing is blocked, so a second instance of the same signal takes the default action and kills the process mid-handler; the classic workaround, re-arming with signal(SIGINT, handler) on entry, trades that for genuine re-entrancy once the disposition is restored. Under BSD semantics the delivered signal is blocked for the duration and neither happens. sigaction() gives the BSD behaviour on every POSIX system, and sa_mask extends it to any sibling signal sharing the handler - which signal() cannot express at all.

Self-Pipe Trick for Passing Data

#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdio.h>
#include <sys/select.h>
#include <unistd.h>

static int signal_pipe[2];

static void handler(int sig) {
    int saved_errno = errno;                      // write() can set errno
    unsigned char byte = (unsigned char)sig;
    ssize_t n = write(signal_pipe[1], &byte, 1);  // async-signal-safe, and cannot block
    (void)n;
    errno = saved_errno;
}

int main(void) {
    if (pipe(signal_pipe) == -1) return 1;
    for (int i = 0; i < 2; i++) {                 // the write end must not block the handler
        fcntl(signal_pipe[i], F_SETFL, fcntl(signal_pipe[i], F_GETFL) | O_NONBLOCK);
    }

    struct sigaction sa;
    sa.sa_handler = handler;
    sigemptyset(&sa.sa_mask);
    sigaddset(&sa.sa_mask, SIGINT);   // one handler serves both signals, so block both
    sigaddset(&sa.sa_mask, SIGTERM);  // while it runs
    sa.sa_flags = SA_RESTART;
    sigaction(SIGINT, &sa, NULL);
    sigaction(SIGTERM, &sa, NULL);

    fd_set fds;
    while (1) {
        FD_ZERO(&fds);
        FD_SET(signal_pipe[0], &fds);

        if (select(signal_pipe[0] + 1, &fds, NULL, NULL, NULL) == -1) {
            if (errno == EINTR) continue;  // SA_RESTART does not restart select()
            return 1;
        }
        unsigned char sig;
        while (read(signal_pipe[0], &sig, 1) == 1) {  // drain everything pending
            printf("Received signal %d\n", sig);      // safe - normal context
            handle_signal(sig);
        }
    }
}

Why this works: The handler's only job is a write() call, which is on the async-signal-safe list because it's a raw system call with no internal buffering or locking to corrupt. Everything that needs non-reentrant functions - parsing, logging, cleanup - runs in the main loop after select()/poll()/epoll() reports the pipe readable, fully outside the signal handler.

The three details around that call are what keep it safe, and each is an omission rather than a mistake, so each survives a code review:

Non-blocking on the write end. A pipe's buffer is finite. If signals arrive faster than the loop drains them, a blocking write() in the handler waits for a reader that cannot run until the handler returns - the process hangs, and whoever is sending the signals decides when. Non-blocking turns the overflow into a dropped byte, which costs nothing here because the loop only needs to know that a signal arrived and drains all of them in one pass.
errno saved and restored. signal-safety(7) makes this a condition of using errno at all from a handler: it is async-signal-safe "provided that the signal handler saves errno on entry and restores its value before returning". Skip it and a handler firing between a failed call and its if (errno == ...) silently replaces the value that code was about to read.
EINTR handled at the select() call. SA_RESTART restarts an interrupted read() or write(); select() and poll() are on the list of calls it never restarts, so a loop built on them has to distinguish "interrupted, go round again" from a real error itself. The shape that fails is select(...); if (FD_ISSET(...)) with the return value ignored: on any failure the descriptor set is left undefined, and the loop reads from a descriptor select() never said was ready.

signalfd for Handler-Free Signal Delivery (Linux)

#include <errno.h>
#include <signal.h>
#include <stdio.h>
#include <sys/signalfd.h>
#include <unistd.h>

int main(void) {
    sigset_t mask;
    sigemptyset(&mask);
    sigaddset(&mask, SIGINT);
    sigaddset(&mask, SIGTERM);
    // Block normal delivery. Do this before creating any thread, so every thread
    // inherits the mask - an unblocked thread would still take the default action.
    if (sigprocmask(SIG_BLOCK, &mask, NULL) == -1) return 1;

    int sfd = signalfd(-1, &mask, 0);
    if (sfd == -1) return 1;

    struct signalfd_siginfo si;
    while (1) {
        ssize_t s = read(sfd, &si, sizeof(si));
        if (s == -1) {
            if (errno == EINTR) continue;
            break;
        }
        if (s == sizeof(si)) {
            printf("Received signal %d\n", si.ssi_signo);  // safe - no handler involved at all
            if (si.ssi_signo == SIGTERM) break;
        }
    }

    close(sfd);
    return 0;
}

Why this works: signalfd removes the signal handler from the picture entirely - blocked signals are delivered as ordinary read events on a file descriptor instead. Because there's no handler, there's no async-signal-safety restriction to violate; every function used here runs in the normal execution context where printf and other non-reentrant calls are always safe.

Blocking the signals is not a preliminary step to signalfd, it is half of the mechanism: a signal still handled the normal way is consumed by that path and never reaches the descriptor. That makes the mask the thing to get right. It is per-thread, so set it before spawning threads or set it in each one - a thread that has not blocked SIGTERM still dies on it, whatever the descriptor is doing. Use pthread_sigmask() rather than sigprocmask() once the program has threads, since sigprocmask() is unspecified there.

Two limits are worth knowing before choosing this over the self-pipe. signalfd is Linux-only, so a portable program keeps the self-pipe or wraps both. And it does not turn standard signals into a queue: several SIGTERMs sent while one is pending still produce one signalfd_siginfo, exactly as ordinary delivery would, so the reader must reconcile state rather than count events.

Testing

Send the relevant signal repeatedly under load (while true; do kill -USR1 $PID; sleep 0.001; done) to expose timing-dependent failures. Where one handler serves several signals, alternate between them rather than repeating one.
Build with ThreadSanitizer (-fsanitize=thread) for the async-signal-safety report, not for race detection. Its report_signal_unsafe option, on by default, reports "violations of async signal-safety (e.g. malloc() call from a signal handler)" - which is this weakness exactly. Its data-race detection will not find it: the handler runs on the thread it interrupted, so TSan sees one thread and has no second one to report a race against.
Statically review every handler for calls to malloc/free, stdio functions, and locking primitives - these are the most common offenders. Follow the transitive calls: a handler calling the program's own log_error() is unsafe if that function reaches fprintf, and nothing at the call site shows it.