diff --git a/src/shared/tests.c b/src/shared/tests.c index 53dda342a4..9dc0980025 100644 --- a/src/shared/tests.c +++ b/src/shared/tests.c @@ -446,7 +446,15 @@ int assert_signal_internal(void) { if (r < 0) return r; - return siginfo.si_status; + /* si_status means different things depending on si_code: + * - CLD_EXITED: si_status is the exit code + * - CLD_KILLED/CLD_DUMPED: si_status is the signal number that killed the process + * We need to return the signal number only if the child was killed by a signal. */ + if (IN_SET(siginfo.si_code, CLD_KILLED, CLD_DUMPED)) + return siginfo.si_status; + + /* Child exited normally, return 0 to indicate no signal was received, regardless of actual exit */ + return 0; } diff --git a/src/test/test-tests.c b/src/test/test-tests.c index 40112a6762..15e36444d3 100644 --- a/src/test/test-tests.c +++ b/src/test/test-tests.c @@ -137,4 +137,33 @@ TEST(ASSERT_OK_OR) { ASSERT_SIGNAL(ASSERT_OK_OR(-1, -2), SIGABRT); } +/* Regression test for issue where assert_signal_internal() wasn't checking si_code before returning + * si_status. + * + * In the bug case, siginfo.si_status has different meanings depending on siginfo.si_code: + * + * - If si_code == CLD_EXITED: si_status is the exit code (0-255) + * - If si_code == CLD_KILLED/CLD_DUMPED: si_status is the signal number + * + * In the bug case where st_code is not checked, exit codes would be confused with signal numbers. For + * example, if a child exits with code 6, it would incorrectly look like SIGABRT. + * + * This test verifies that exit codes are NOT confused with signal numbers, even when the exit code + * numerically matches a signal number. + */ +TEST(ASSERT_SIGNAL_exit_code_vs_signal) { + /* These exit codes numerically match common signal numbers, but ASSERT_SIGNAL should correctly + * identify them as exit codes (si_code==CLD_EXITED), not signals. The inner ASSERT_SIGNAL expects a + * signal but gets an exit code, so it should fail (aborting with SIGABRT), which the outer + * ASSERT_SIGNAL then catches. */ + + ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(6), SIGABRT), SIGABRT); /* 6 = SIGABRT */ + ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(9), SIGKILL), SIGABRT); /* 9 = SIGKILL */ + ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(11), SIGSEGV), SIGABRT); /* 11 = SIGSEGV */ + ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(15), SIGTERM), SIGABRT); /* 15 = SIGTERM */ + + /* _exit(0) should not be confused with any signal */ + ASSERT_SIGNAL(ASSERT_SIGNAL(_exit(0), SIGABRT), SIGABRT); +} + DEFINE_TEST_MAIN(LOG_INFO);