]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
tree-wide: only use .si_pid field in siginfo_t, if .si_code indicates that's safe
authorLennart Poettering <lennart@poettering.net>
Thu, 29 May 2025 06:22:07 +0000 (08:22 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sat, 31 May 2025 12:48:37 +0000 (14:48 +0200)
Fixes: #37498
src/basic/log.c
src/basic/signal-util.h
src/core/crash-handler.c
src/coredump/coredumpctl.c
src/journal/journald-manager.c
src/shared/common-signal.c
src/udev/udev-watch.c

index be036d6e738cb5acc0d2b38870551e923ea39193..bd338d8294d86c3fb01a09f059f54f381a22709e 100644 (file)
@@ -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);
index 5a253583a7665b93e23e7c10fb649e45a03f30ee..c45b9934d021cf1cef2d7dd2875708769c719a40 100644 (file)
@@ -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);
+}
index 41826a1f3e06fc9331cf1ce3f6d472c9b0cd3d0f..b2b09d493aa0886ca6293f9abc6cc7a8ab1e68b0 100644 (file)
@@ -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));
index 5dbd7cfa8d255383a9a9630aa841c84d5c474e25..a8d81955c0acf7f9eb3da17b2151ebfa6f53d58a 100644 (file)
@@ -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);
 }
 
index 55aa8bc30960391698024c0c496b6c869741cd18..bf3befee48383150ef1187a7dcde8fc1407b5428 100644 (file)
@@ -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);
index cbc17778d76b97a5d41b969bb76b8993c72b5780..3eeb81029223e27c9d9983c167d96d2e22645941 100644 (file)
@@ -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) {
index 93d51d461b8e781b86dc23f0ccb5a9fe1b9e4698..5f77b4e70807c8ed571e7871cc81dc2659f86a01 100644 (file)
@@ -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;