From: Lennart Poettering Date: Thu, 29 May 2025 06:22:07 +0000 (+0200) Subject: tree-wide: only use .si_pid field in siginfo_t, if .si_code indicates that's safe X-Git-Tag: v258-rc1~433 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1105f8d20fa6ad511b736b9244684aa596697c8f;p=thirdparty%2Fsystemd.git tree-wide: only use .si_pid field in siginfo_t, if .si_code indicates that's safe Fixes: #37498 --- diff --git a/src/basic/log.c b/src/basic/log.c index be036d6e738..bd338d8294d 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -1507,7 +1507,7 @@ DEFINE_STRING_TABLE_LOOKUP(log_target, LogTarget); void log_received_signal(int level, const struct signalfd_siginfo *si) { assert(si); - if (pid_is_valid(si->ssi_pid)) { + if (si_code_from_process(si->ssi_code) && pid_is_valid(si->ssi_pid)) { _cleanup_free_ char *p = NULL; (void) pid_get_comm(si->ssi_pid, &p); diff --git a/src/basic/signal-util.h b/src/basic/signal-util.h index 5a253583a76..c45b9934d02 100644 --- a/src/basic/signal-util.h +++ b/src/basic/signal-util.h @@ -71,3 +71,25 @@ extern const struct sigaction sigaction_default; extern const struct sigaction sigaction_nop_nocldstop; int parse_signo(const char *s, int *ret); + +static inline bool si_code_from_process(int si_code) { + /* Returns true if the .si_code field of siginfo_t or the .ssi_code field of struct signalfd_siginfo + * indicate that the signal originates from a userspace process, and hence the .si_pid/.ssi_pid field + * is valid. This check is not obvious, since on one hand SI_USER/SI_QUEUE are supposed to be the + * values that kill() and sigqueue() set, and that's documented in sigaction(2), but on the other + * hand rt_sigqueueinfo(2) says userspace can actually set any value below zero. Hence check for + * either. + * + * Also quoting POSIX: + * + * "On systems not supporting the XSI option, the si_pid and si_uid members of siginfo_t are only + * required to be valid when si_code is SI_USER or SI_QUEUE. On XSI-conforming systems, they are also + * valid for all si_code values less than or equal to 0; however, it is unspecified whether SI_USER + * and SI_QUEUE have values less than or equal to zero, and therefore XSI applications should check + * whether si_code has the value SI_USER or SI_QUEUE or is less than or equal to 0 to tell whether + * si_pid and si_uid are valid." + * + * From: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html */ + + return si_code < 0 || IN_SET(si_code, SI_USER, SI_QUEUE); +} diff --git a/src/core/crash-handler.c b/src/core/crash-handler.c index 41826a1f3e0..b2b09d493aa 100644 --- a/src/core/crash-handler.c +++ b/src/core/crash-handler.c @@ -104,7 +104,11 @@ _noreturn_ static void crash(int sig, siginfo_t *siginfo, void *context) { siginfo_t status; if (siginfo) { - if (siginfo->si_pid == 0) + if (!si_code_from_process(siginfo->si_code)) + log_struct(LOG_EMERG, + LOG_MESSAGE("Caught <%s>.", signal_to_string(sig)), + LOG_MESSAGE_ID(SD_MESSAGE_CRASH_UNKNOWN_SIGNAL_STR)); + else if (siginfo->si_pid == 0) log_struct(LOG_EMERG, LOG_MESSAGE("Caught <%s>, from unknown sender process.", signal_to_string(sig)), LOG_MESSAGE_ID(SD_MESSAGE_CRASH_UNKNOWN_SIGNAL_STR)); diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 5dbd7cfa8d2..a8d81955c0a 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -1170,7 +1170,9 @@ static void sigterm_handler(int signal, siginfo_t *info, void *ucontext) { /* If the sender is not us, propagate the signal to all processes in * the same process group */ - if (pid_is_valid(info->si_pid) && info->si_pid != getpid_cached()) + if (si_code_from_process(info->si_code) && + pid_is_valid(info->si_pid) && + info->si_pid != getpid_cached()) (void) kill(0, signal); } diff --git a/src/journal/journald-manager.c b/src/journal/journald-manager.c index 55aa8bc3096..bf3befee483 100644 --- a/src/journal/journald-manager.c +++ b/src/journal/journald-manager.c @@ -58,6 +58,7 @@ #include "process-util.h" #include "rm-rf.h" #include "set.h" +#include "signal-util.h" #include "socket-netlink.h" #include "socket-util.h" #include "stdio-util.h" @@ -1645,9 +1646,15 @@ void manager_full_flush(Manager *m) { static int dispatch_sigusr1(sd_event_source *es, const struct signalfd_siginfo *si, void *userdata) { Manager *m = ASSERT_PTR(userdata); + assert(si); + + if (!si_code_from_process(si->ssi_code)) { + log_warning("Received SIGUSR1 with unexpected .si_code %i, ignoring.", si->ssi_code); + return 0; + } if (m->namespace) { - log_error("Received SIGUSR1 signal from PID %u, but flushing runtime journals not supported for namespaced instances.", si->ssi_pid); + log_warning("Received SIGUSR1 signal from PID %u, but flushing runtime journals not supported for namespaced instances, ignoring.", si->ssi_pid); return 0; } @@ -1681,6 +1688,12 @@ void manager_full_rotate(Manager *m) { static int dispatch_sigusr2(sd_event_source *es, const struct signalfd_siginfo *si, void *userdata) { Manager *m = ASSERT_PTR(userdata); + assert(si); + + if (!si_code_from_process(si->ssi_code)) { + log_warning("Received SIGUSR2 with unexpected .si_code %i, ignoring.", si->ssi_code); + return 0; + } log_info("Received SIGUSR2 signal from PID %u, as request to rotate journal, rotating.", si->ssi_pid); manager_full_rotate(m); @@ -1785,6 +1798,12 @@ void manager_full_sync(Manager *m, bool wait) { static int dispatch_sigrtmin1(sd_event_source *es, const struct signalfd_siginfo *si, void *userdata) { Manager *m = ASSERT_PTR(userdata); + assert(si); + + if (!si_code_from_process(si->ssi_code)) { + log_warning("Received SIGRTMIN1 with unexpected .si_code %i, ignoring.", si->ssi_code); + return 0; + } log_debug("Received SIGRTMIN1 signal from PID %u, as request to sync.", si->ssi_pid); manager_full_sync(m, /* wait = */ false); diff --git a/src/shared/common-signal.c b/src/shared/common-signal.c index cbc17778d76..3eeb8102922 100644 --- a/src/shared/common-signal.c +++ b/src/shared/common-signal.c @@ -21,6 +21,11 @@ int sigrtmin18_handler(sd_event_source *s, const struct signalfd_siginfo *si, vo assert(s); assert(si); + if (!si_code_from_process(si->ssi_code)) { + log_notice("Received control signal %s with unexpected .si_code %i, ignoring.", signal_to_string(si->ssi_signo), si->ssi_code); + return 0; + } + (void) pid_get_comm(si->ssi_pid, &comm); if (si->ssi_code != SI_QUEUE) { diff --git a/src/udev/udev-watch.c b/src/udev/udev-watch.c index 93d51d461b8..5f77b4e7080 100644 --- a/src/udev/udev-watch.c +++ b/src/udev/udev-watch.c @@ -23,6 +23,7 @@ #include "process-util.h" #include "rm-rf.h" #include "set.h" +#include "signal-util.h" #include "stdio-util.h" #include "string-util.h" #include "udev-manager.h" @@ -611,6 +612,11 @@ int manager_remove_watch(Manager *manager, sd_device *dev) { static int on_sigusr1(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { UdevWorker *worker = ASSERT_PTR(userdata); + if (!si_code_from_process(si->ssi_code)) { + log_debug("Received SIGUSR1 with unexpected .si_code %i, ignoring.", si->ssi_code); + return 0; + } + if ((pid_t) si->ssi_pid != worker->manager_pid) { log_debug("Received SIGUSR1 from unexpected process [%"PRIu32"], ignoring.", si->ssi_pid); return 0;