From 998a356e49da530912ae4dc7d4472c44a03baeab Mon Sep 17 00:00:00 2001 From: Harshith Allamsetti Date: Sun, 6 Sep 2026 15:54:20 -0700 Subject: [PATCH] shared: add crash handler that prints a backtrace Print the call stack via backtrace() on fatal signals (SIGSEGV, SIGABRT, SIGFPE, SIGILL, SIGBUS), then re-raise with default disposition so core dumps and kill status are preserved. Lives in shared/ so the CLI, libnvme, discoverd and nvmf-autoconnect can all use it. libnvme installs the handler automatically via a constructor so every consumer gets crash backtraces without code changes. The nvme CLI also calls shr_install_crash_handler() at startup as a belt-and-suspenders measure. Per-signal skip: shr_install_crash_handler() leaves a signal alone if the embedding application has already installed a handler for it (e.g. Sentry, systemd-coredump, SA_SIGINFO with richer context), but still installs handlers for the rest. backtrace() may lazy-load libgcc_s.so on its first call via malloc(), which is unsafe inside a signal handler if the heap is already corrupted. shr_warmup_backtrace() runs before any handler is installed so the lazy-load happens safely during normal startup. Closes: #3716 Signed-off-by: Harshith Allamsetti --- libnvme/src/meson.build | 1 + libnvme/src/nvme/crash-handler.c | 13 ++ meson.build | 3 + shared/crash-util-linux.c | 188 ++++++++++++++++++++++ shared/crash-util.h | 43 +++++ shared/meson.build | 1 + shared/tests/meson.build | 11 ++ shared/tests/test-crash-util.c | 268 +++++++++++++++++++++++++++++++ src/logging.c | 12 ++ src/logging.h | 3 + src/nvme.c | 5 + 11 files changed, 548 insertions(+) create mode 100644 libnvme/src/nvme/crash-handler.c create mode 100644 shared/crash-util-linux.c create mode 100644 shared/crash-util.h create mode 100644 shared/tests/test-crash-util.c diff --git a/libnvme/src/meson.build b/libnvme/src/meson.build index 0e5c1def7c..1f0b1f5df2 100644 --- a/libnvme/src/meson.build +++ b/libnvme/src/meson.build @@ -17,6 +17,7 @@ sources = [ if host_system == 'linux' sources += [ 'nvme/attr-accessors-custom-linux.c', + 'nvme/crash-handler.c', 'nvme/ioctl-linux.c', 'nvme/lib-linux.c', 'nvme/mem-linux.c', diff --git a/libnvme/src/nvme/crash-handler.c b/libnvme/src/nvme/crash-handler.c new file mode 100644 index 0000000000..33fc4c2c19 --- /dev/null +++ b/libnvme/src/nvme/crash-handler.c @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/* + * This file is part of libnvme. + * + * Initialize the crash handler when libnvme is loaded. + */ + +#include + +static __attribute__((constructor)) void libnvme_init_crash_handler(void) +{ + shr_install_crash_handler(); +} diff --git a/meson.build b/meson.build index 03f061029d..5c187721fe 100644 --- a/meson.build +++ b/meson.build @@ -650,6 +650,9 @@ if want_nvme endif else link_args_list = ['-ldl'] + if cc.has_link_argument('-rdynamic') + link_args_list += '-rdynamic' + endif endif nvme_exe = executable( diff --git a/shared/crash-util-linux.c b/shared/crash-util-linux.c new file mode 100644 index 0000000000..131b62ddbf --- /dev/null +++ b/shared/crash-util-linux.c @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/* + * This file is part of nvme-cli. + * + * Linux/macOS implementation of the crash handler. Uses backtrace() and + * backtrace_symbols_fd() from , both async-signal-safe, so the + * whole handler performs only write()-based I/O (see signal-safety(7)). + */ +#include +#include +#include +#include +#include + +#include + +#include "crash-util.h" + +#define SHR_CRASH_BT_DEPTH 64 + +static void *shr_crash_bt[SHR_CRASH_BT_DEPTH]; + +struct shr_crash_sig { + int sig; + const char *name; +}; + +static const struct shr_crash_sig shr_crash_signals[] = { + { SIGSEGV, "SIGSEGV" }, + { SIGABRT, "SIGABRT" }, + { SIGFPE, "SIGFPE" }, + { SIGILL, "SIGILL" }, +#ifdef SIGBUS + { SIGBUS, "SIGBUS" }, +#endif +}; + +/* + * strlen() is not async-signal-safe, so measure the length inline and emit + * the bytes with a single write(). Assigning to a discarded variable also + * keeps warn_unused_result (enabled by _FORTIFY_SOURCE) quiet. + */ +static void crash_write_str(int fd, const char *s) +{ + size_t len = 0; + ssize_t ret; + + if (!s) + return; + + while (s[len]) + len++; + + ret = write(fd, s, len); + (void)ret; +} + +/* Decimal conversion without snprintf(): stdio locks FILE internals. */ +static void crash_write_uint(int fd, unsigned int n) +{ + char buf[10]; /* enough for any 32-bit unsigned int */ + int i = sizeof(buf); + ssize_t ret; + + do { + buf[--i] = '0' + (n % 10); + n /= 10; + } while (n && i); + + ret = write(fd, buf + i, sizeof(buf) - i); + (void)ret; +} + +static const char *crash_sig_name(int sig) +{ + size_t i; + + for (i = 0; i < sizeof(shr_crash_signals) / sizeof(shr_crash_signals[0]); i++) + if (shr_crash_signals[i].sig == sig) + return shr_crash_signals[i].name; + + return "SIG?"; +} + +void shr_print_backtrace(int fd) +{ + int n = backtrace(shr_crash_bt, SHR_CRASH_BT_DEPTH); + + crash_write_str(fd, "backtrace:\n"); + backtrace_symbols_fd(shr_crash_bt, n, fd); + crash_write_str(fd, "\n"); +} + +/* + * Warm up backtrace(): the first call in a process may trigger lazy-loading + * of libgcc_s.so via an internal malloc(), which is unsafe inside a signal + * handler. Calling it once here, during normal startup, ensures the library + * is already mapped before any crash handler runs. + */ +void shr_warmup_backtrace(void) +{ + /* + * Call backtrace() with a small stack buffer. We discard the result - + * all we need is to trigger the lazy-load. backtrace() is async- + * signal-safe, so this is always safe to call. + */ + void *dummy[1]; + + backtrace(dummy, 1); +} + +static void shr_crash_handler(int sig) +{ + int fd = STDERR_FILENO; + + /* + * The reporting below writes to stderr, which may be a pipe whose read + * end has already gone away (e.g. 'nvme ... 2>&1 | head'). Doing a + * write() to such a pipe raises SIGPIPE, whose default action is to + * terminate -- which would mask the very crash we are reporting. Ignore + * SIGPIPE for the duration of the handler so write() instead returns + * EPIPE harmlessly (signal() is async-signal-safe). + */ + signal(SIGPIPE, SIG_IGN); + + crash_write_str(fd, "fatal: signal "); + crash_write_str(fd, crash_sig_name(sig)); + crash_write_str(fd, " (pid "); + crash_write_uint(fd, getpid()); + crash_write_str(fd, ")\n"); + + shr_print_backtrace(fd); + + /* + * SA_RESETHAND restored the default disposition before calling us, so + * this kills the process with the default action for the signal rather + * than returning into (possibly corrupted) user code. Sanity-check the + * return in case a platform refuses to deliver the signal again. + */ + if (raise(sig) != 0) + _exit(EXIT_FAILURE); +} + +int shr_install_crash_handler(void) +{ + struct sigaction act = { 0 }, old_act = { 0 }; + size_t i; + int ret = 0; + + /* + * Warm up backtrace() before installing signal handlers. The first call + * may trigger lazy-loading of libgcc_s.so via an internal malloc(), + * which is unsafe inside a signal handler. See shr_warmup_backtrace() + * and backtrace(3) notes. + */ + shr_warmup_backtrace(); + + sigemptyset(&act.sa_mask); + act.sa_handler = shr_crash_handler; + /* + * SA_RESETHAND: reset to the default disposition before entering the + * handler so the re-raise() above takes the default action, and a fault + * inside the handler cannot recurse forever. + * SA_NODEFER: do not block the signal while the handler runs. + */ + act.sa_flags = SA_RESETHAND | SA_NODEFER; + + /* + * Decide per signal, independently, whether to install. This respects + * applications that embed libnvme and bring their own crash reporting + * for a specific signal (e.g. Sentry, systemd-coredump, SA_SIGINFO + * handlers with richer context) without giving up backtraces for every + * other signal in the set just because one of them is already claimed. + */ + for (i = 0; i < sizeof(shr_crash_signals) / sizeof(shr_crash_signals[0]); i++) { + if (sigaction(shr_crash_signals[i].sig, NULL, &old_act) == -1) { + ret = ret ? ret : -errno; + continue; + } + if (old_act.sa_handler != SIG_DFL) + continue; + + if (sigaction(shr_crash_signals[i].sig, &act, NULL) == -1) + ret = ret ? ret : -errno; + } + + return ret; +} \ No newline at end of file diff --git a/shared/crash-util.h b/shared/crash-util.h new file mode 100644 index 0000000000..454a853f54 --- /dev/null +++ b/shared/crash-util.h @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/* + * This file is part of nvme-cli. + */ +#pragma once + +/** + * shr_install_crash_handler() - Install fatal-signal handlers that dump the + * call stack to stderr + * + * Installs handlers for the fatal signals (SIGSEGV, SIGABRT, SIGFPE, SIGILL + * and, where defined, SIGBUS) that print a short diagnostic plus the current + * call stack (via backtrace_symbols_fd(), which is async-signal-safe) and then + * re-raise the signal with its default disposition, so core dumps and the + * process' kill status are preserved for debuggers and CI. + * + * Return: 0 on success, negative errno on failure. + */ +int shr_install_crash_handler(void); + +/** + * shr_warmup_backtrace() - Pre-load libgcc_s.so used by backtrace() + * + * The first call to backtrace() in a process may internally call malloc() + * to lazy-load libgcc_s.so (the unwinding library). That malloc() is not + * safe inside a signal handler, so this function should be called once + * during normal startup, before shr_install_crash_handler(), to trigger + * the lazy-load ahead of time. shr_install_crash_handler() calls this + * itself, so callers normally do not need to invoke it directly. Only use + * this if you want to capture backtraces from a signal handler in code + * that does not go through shr_install_crash_handler(). + */ +void shr_warmup_backtrace(void); + +/** + * shr_print_backtrace() - Print the calling thread's call stack to @fd + * @fd: File descriptor to write to, e.g. STDERR_FILENO. + * + * Always prints a "backtrace:" header line, then one line per stack frame as + * produced by backtrace_symbols_fd(). Safe to call at any time; also used + * internally by the crash handler. + */ +void shr_print_backtrace(int fd); \ No newline at end of file diff --git a/shared/meson.build b/shared/meson.build index 2dc564d6ac..f710de6b20 100644 --- a/shared/meson.build +++ b/shared/meson.build @@ -39,6 +39,7 @@ if host_system == 'windows' ] else sources += [ + 'crash-util-linux.c', 'fs-util-linux.c', 'net-util-linux.c', 'crypto-util-linux.c', diff --git a/shared/tests/meson.build b/shared/tests/meson.build index d8a23382fe..beb21dbce9 100644 --- a/shared/tests/meson.build +++ b/shared/tests/meson.build @@ -288,6 +288,17 @@ test_sig_util = executable( test('shared - sig-util', test_sig_util) +test_crash_util = executable( + 'test-crash-util', + ['test-crash-util.c'], + dependencies: [ + config_dep, + shared_dep, + ], +) + +test('shared - crash-util', test_crash_util) + test_assert_util = executable( 'test-assert-util', ['test-assert-util.c'], diff --git a/shared/tests/test-crash-util.c b/shared/tests/test-crash-util.c new file mode 100644 index 0000000000..a66abbfe60 --- /dev/null +++ b/shared/tests/test-crash-util.c @@ -0,0 +1,268 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/* + * This file is part of nvme-cli. + * + * Tests for shared/crash-util: + * - shr_install_crash_handler() succeeds + * - shr_print_backtrace() emits a "backtrace:" header + * - a child raising SIGSEGV prints the diagnostic + backtrace to stderr and + * dies with the default disposition (WIFSIGNALED / WTERMSIG == SIGSEGV), + * proving the crash handler preserves kill-status/core semantics. + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +static bool check_bool(const char *name, bool got) +{ + printf(" - %s [%s]\n", name, got ? "PASS" : "FAIL"); + return got; +} + +static bool test_install(void) +{ + bool pass = true; + + printf("test_install:\n"); + + pass &= check_bool("handler installs successfully", + shr_install_crash_handler() == 0); + + return pass; +} + +static bool test_install_skips_existing_handler(void) +{ + bool pass = true; + struct sigaction pre = { 0 }, post = { 0 }, post_abrt = { 0 }; + + printf("test_install_skips_existing_handler:\n"); + + /* + * Pretend the application already installed a custom SIGSEGV handler + * (e.g. Sentry, systemd-coredump). shr_install_crash_handler() must + * detect that and skip SIGSEGV while still installing handlers for the + * other signals. Verify by checking SIGSEGV (preserved) and SIGABRT + * (installed). + */ + pre.sa_handler = (void (*)(int))(uintptr_t)0xdeadbeef; + pre.sa_flags = SA_RESTART; + pass &= check_bool("preinstall SIGSEGV handler succeeds", + sigaction(SIGSEGV, &pre, NULL) == 0); + + pass &= check_bool("install returns success", + shr_install_crash_handler() == 0); + + pass &= check_bool("SIGSEGV handler preserved", + sigaction(SIGSEGV, NULL, &post) == 0 && + post.sa_handler == pre.sa_handler); + + /* SIGABRT was not claimed by the app, so ours should be installed. */ + pass &= check_bool("SIGABRT handler installed", + sigaction(SIGABRT, NULL, &post_abrt) == 0 && + post_abrt.sa_handler != SIG_DFL); + + /* Restore the default handler so later tests can install ours cleanly. */ + struct sigaction dfl; + memset(&dfl, 0, sizeof(dfl)); + dfl.sa_handler = SIG_DFL; + sigaction(SIGSEGV, &dfl, NULL); + sigaction(SIGABRT, &dfl, NULL); + + return pass; +} + +/* + * Must run before any other test in this binary calls backtrace() for + * any reason -- the whole point is to prove shr_install_crash_handler() + * safely warms up backtrace() on a genuinely cold process, where the + * very first ever call to backtrace() happens inside a signal handler + * if the warmup is missing or broken. If this ran after + * test_warmup_backtrace() or test_manual_backtrace(), a forked child + * would inherit an already-warmed-up process via copy-on-write and this + * test could pass even with a regression that removes the internal + * warmup call. + */ +static bool test_crash_sigsegv_cold(void) +{ + bool pass = true; + int p[2]; + pid_t pid; + int status = 0; + char buf[8192]; + ssize_t n; + size_t len = 0; + + printf("test_crash_sigsegv_cold:\n"); + + pass &= check_bool("pipe", pipe(p) == 0); + if (!pass) + return pass; + + fflush(NULL); + + pid = fork(); + if (pid == 0) { + close(p[0]); + dup2(p[1], STDERR_FILENO); + close(p[1]); + + /* + * shr_install_crash_handler() must itself call + * shr_warmup_backtrace() -- this process has never called + * backtrace() before this line. + */ + if (shr_install_crash_handler()) + _exit(EXIT_FAILURE); + raise(SIGSEGV); + _exit(EXIT_FAILURE); + } + + close(p[1]); + + while (len < sizeof(buf) - 1 && + (n = read(p[0], buf + len, sizeof(buf) - 1 - len)) > 0) + len += n; + buf[len] = '\0'; + close(p[0]); + + pass &= check_bool("child killed by SIGSEGV", + waitpid(pid, &status, 0) == pid && + WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV); + pass &= check_bool("stderr has backtrace on cold process", + strstr(buf, "backtrace:") != NULL); + + return pass; +} + +static bool test_warmup_backtrace(void) +{ + bool pass = true; + + printf("test_warmup_backtrace:\n"); + + /* + * The warmup must be safe to call any number of times and must not + * change process state in an observable way. We exercise it before + * installing the crash handler (the path an external caller would + * use) and again after, to make sure it is idempotent. + */ + pass &= check_bool("call before install does not crash", + (shr_warmup_backtrace(), true)); + pass &= check_bool("install after warmup still works", + shr_install_crash_handler() == 0); + pass &= check_bool("call after install is still safe", + (shr_warmup_backtrace(), true)); + + return pass; +} + +static bool test_manual_backtrace(void) +{ + bool pass = true; + int p[2]; + char buf[4096]; + ssize_t n; + + printf("test_manual_backtrace:\n"); + + pass &= check_bool("pipe", pipe(p) == 0); + if (!pass) + return pass; + + shr_print_backtrace(p[1]); + close(p[1]); + + n = read(p[0], buf, sizeof(buf) - 1); + close(p[0]); + + pass &= check_bool("read produced output", n > 0); + if (n <= 0) + return pass; + + buf[n] = '\0'; + pass &= check_bool("contains backtrace header", + strstr(buf, "backtrace:") != NULL); + + return pass; +} + +static bool test_crash_sigsegv(void) +{ + bool pass = true; + int p[2]; + pid_t pid; + int status = 0; + char buf[8192]; + ssize_t n; + size_t len = 0; + + printf("test_crash_sigsegv:\n"); + + pass &= check_bool("pipe", pipe(p) == 0); + if (!pass) + return pass; + + /* Flush before fork so the child does not duplicate buffered output. */ + fflush(NULL); + + pid = fork(); + if (pid == 0) { + close(p[0]); + dup2(p[1], STDERR_FILENO); + close(p[1]); + + if (shr_install_crash_handler()) + _exit(EXIT_FAILURE); + raise(SIGSEGV); + + /* The handler re-raises with the default disposition, so we should + * never get here unless the handler is broken. */ + _exit(EXIT_FAILURE); + } + + close(p[1]); + + /* Read until EOF: the child's stderr stays open until it dies, so a + * short read is no reason to stop -- and closing the pipe early would + * SIGPIPE-kill the child mid-backtrace. */ + while (len < sizeof(buf) - 1 && + (n = read(p[0], buf + len, sizeof(buf) - 1 - len)) > 0) + len += n; + buf[len] = '\0'; + close(p[0]); + + pass &= check_bool("child killed by SIGSEGV", + waitpid(pid, &status, 0) == pid && + WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV); + pass &= check_bool("stderr has fatal diagnostic", + strstr(buf, "fatal: signal SIGSEGV") != NULL); + pass &= check_bool("stderr has backtrace", + strstr(buf, "backtrace:") != NULL); + + return pass; +} + +int main(void) +{ + bool pass = true; + + /* Must be first: see comment on test_crash_sigsegv_cold(). */ + pass &= test_crash_sigsegv_cold(); + + pass &= test_warmup_backtrace(); + pass &= test_install(); + pass &= test_install_skips_existing_handler(); + pass &= test_manual_backtrace(); + pass &= test_crash_sigsegv(); + + fflush(stdout); + exit(pass ? EXIT_SUCCESS : EXIT_FAILURE); +} \ No newline at end of file diff --git a/src/logging.c b/src/logging.c index 90ada65be6..3d9c5e127b 100644 --- a/src/logging.c +++ b/src/logging.c @@ -6,11 +6,13 @@ #include #include #include +#include #include #include #include +#include #include #include "logging.h" @@ -24,6 +26,16 @@ struct submit_data { int log_level; static struct submit_data sb; +int nvme_install_crash_handler(void) +{ + return shr_install_crash_handler(); +} + +void nvme_show_crash_backtrace(void) +{ + shr_print_backtrace(STDERR_FILENO); +} + bool is_printable_at_level(int level) { return ((log_level >= level) && diff --git a/src/logging.h b/src/logging.h index 3b22d57ecf..0009b48f69 100644 --- a/src/logging.h +++ b/src/logging.h @@ -40,3 +40,6 @@ void nvme_mi_submit_exit(struct libnvme_mi_ep *ep, __u8 type, bool is_printable_at_level(int level); int map_log_level(int verbose, bool quiet); + +int nvme_install_crash_handler(void); +void nvme_show_crash_backtrace(void); diff --git a/src/nvme.c b/src/nvme.c index 54022fd7fa..463ecfadca 100644 --- a/src/nvme.c +++ b/src/nvme.c @@ -49,6 +49,7 @@ #include "cleanup.h" #include "global-config.h" #include "global-ctx.h" +#include "logging.h" #include "nvme-print.h" #include "plugin.h" @@ -173,6 +174,10 @@ int main(int argc, char **argv) if (err) return err; + err = nvme_install_crash_handler(); + if (err) + return err; + err = handle_plugin(argc, argv, nvme.extensions); if (err == -ENOTTY) general_help(&builtin, NULL);