From 897448bd3753eab2c7b411221cfc33b283ae67a5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Sep 2022 11:39:25 +0200 Subject: [PATCH] sd-event: if signal nr has high bit set sd_event_add_signal() auto-block it via sigprocmask() So far we expected callers to block the signals manually. Which is usually a good idea, since they should do that before forking off threads and similar. But let's add a mode where we automatically block it for the caller, to simplify things. --- man/rules/meson.build | 4 +- man/sd_event_add_signal.xml | 41 ++++++++------- src/libsystemd/sd-event/event-source.h | 1 + src/libsystemd/sd-event/sd-event.c | 71 +++++++++++++++++++++++--- src/systemd/sd-event.h | 2 + 5 files changed, 91 insertions(+), 28 deletions(-) diff --git a/man/rules/meson.build b/man/rules/meson.build index 2925dadc1e..7f4a42b139 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -555,7 +555,9 @@ manpages = [ ''], ['sd_event_add_signal', '3', - ['sd_event_signal_handler_t', 'sd_event_source_get_signal'], + ['SD_EVENT_SIGNAL_PROCMASK', + 'sd_event_signal_handler_t', + 'sd_event_source_get_signal'], ''], ['sd_event_add_time', '3', diff --git a/man/sd_event_add_signal.xml b/man/sd_event_add_signal.xml index b2aaff87c1..3e8536e961 100644 --- a/man/sd_event_add_signal.xml +++ b/man/sd_event_add_signal.xml @@ -19,6 +19,7 @@ sd_event_add_signal sd_event_source_get_signal sd_event_signal_handler_t + SD_EVENT_SIGNAL_PROCMASK Add a UNIX process signal event source to an event loop @@ -30,6 +31,8 @@ typedef struct sd_event_source sd_event_source; + SD_EVENT_SIGNAL_PROCMASK + typedef int (*sd_event_signal_handler_t) sd_event_source *s @@ -50,30 +53,26 @@ int sd_event_source_get_signal sd_event_source *source - Description - sd_event_add_signal() adds a new UNIX - process signal event source to an event loop. The event loop - object is specified in the event parameter, - and the event source object is returned in the - source parameter. The - signal parameter specifies the numeric - signal to be handled (see sd_event_add_signal() adds a new UNIX process signal event source to an event + loop. The event loop object is specified in the event parameter, and the event + source object is returned in the source parameter. The + signal parameter specifies the numeric signal to be handled (see signal7). The handler parameter is a function to call when the signal is received or NULL. The handler function will be passed the userdata pointer, which may be chosen freely by the caller. The handler also receives a pointer to a signalfd_siginfo structure containing information about the received signal. See - signalfd2 - for further information. The handler may return negative to signal an error (see below), other return - values are ignored. If handler is NULL, a default handler - that calls + signalfd2 for + further information. The handler may return negative to signal an error (see below), other return values + are ignored. If handler is NULL, a default handler that calls sd_event_exit3 will be used. @@ -81,14 +80,18 @@ threads before this function is called (using sigprocmask2 or pthread_sigmask3). - - By default, the event source is enabled permanently - (SD_EVENT_ON), but this may be changed with + project='man-pages'>pthread_sigmask3). For + convenience, if the special flag SD_EVENT_SIGNAL_PROCMASK is ORed into the specified + signal the signal will be automatically masked as necessary, for the calling thread. Note that this only + works reliably if the signal is already masked in all other threads of the process, or if there are no + other threads at the moment of invocation. + + By default, the event source is enabled permanently (SD_EVENT_ON), but this + may be changed with sd_event_source_set_enabled3. - If the handler function returns a negative error code, it will either be disabled after the - invocation, even if the SD_EVENT_ON mode was requested before, or it will cause the - loop to terminate, see + If the handler function returns a negative error code, it will either be disabled after the invocation, + even if the SD_EVENT_ON mode was requested before, or it will cause the loop to + terminate, see sd_event_source_set_exit_on_failure3. diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 74cbc26962..6092652d0f 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -99,6 +99,7 @@ struct sd_event_source { sd_event_signal_handler_t callback; struct signalfd_siginfo siginfo; int sig; + bool unblock; } signal; struct { sd_event_child_handler_t callback; diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 6f99c5f0cd..890b62b1f9 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -813,6 +813,7 @@ static void event_source_time_prioq_remove( static void source_disconnect(sd_event_source *s) { sd_event *event; + int r; assert(s); @@ -853,6 +854,20 @@ static void source_disconnect(sd_event_source *s) { s->event->signal_sources[s->signal.sig] = NULL; event_gc_signal_data(s->event, &s->priority, s->signal.sig); + + if (s->signal.unblock) { + sigset_t new_ss; + + if (sigemptyset(&new_ss) < 0) + log_debug_errno(errno, "Failed to reset signal set, ignoring: %m"); + else if (sigaddset(&new_ss, s->signal.sig) < 0) + log_debug_errno(errno, "Failed to add signal %i to signal mask, ignoring: %m", s->signal.sig); + else { + r = pthread_sigmask(SIG_UNBLOCK, &new_ss, NULL); + if (r != 0) + log_debug_errno(r, "Failed to unblock signal %i, ignoring: %m", s->signal.sig); + } + } } break; @@ -1328,23 +1343,38 @@ _public_ int sd_event_add_signal( _cleanup_(source_freep) sd_event_source *s = NULL; struct signal_data *d; + sigset_t new_ss; + bool block_it; int r; assert_return(e, -EINVAL); assert_return(e = event_resolve(e), -ENOPKG); - assert_return(SIGNAL_VALID(sig), -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + /* Let's make sure our special flag stays outside of the valid signal range */ + assert_cc(_NSIG < SD_EVENT_SIGNAL_PROCMASK); + + if (sig & SD_EVENT_SIGNAL_PROCMASK) { + sig &= ~SD_EVENT_SIGNAL_PROCMASK; + assert_return(SIGNAL_VALID(sig), -EINVAL); + + block_it = true; + } else { + assert_return(SIGNAL_VALID(sig), -EINVAL); + + r = signal_is_blocked(sig); + if (r < 0) + return r; + if (r == 0) + return -EBUSY; + + block_it = false; + } + if (!callback) callback = signal_exit_callback; - r = signal_is_blocked(sig); - if (r < 0) - return r; - if (r == 0) - return -EBUSY; - if (!e->signal_sources) { e->signal_sources = new0(sd_event_source*, _NSIG); if (!e->signal_sources) @@ -1363,9 +1393,34 @@ _public_ int sd_event_add_signal( e->signal_sources[sig] = s; + if (block_it) { + sigset_t old_ss; + + if (sigemptyset(&new_ss) < 0) + return -errno; + + if (sigaddset(&new_ss, sig) < 0) + return -errno; + + r = pthread_sigmask(SIG_BLOCK, &new_ss, &old_ss); + if (r != 0) + return -r; + + r = sigismember(&old_ss, sig); + if (r < 0) + return -errno; + + s->signal.unblock = !r; + } else + s->signal.unblock = false; + r = event_make_signal_data(e, sig, &d); - if (r < 0) + if (r < 0) { + if (s->signal.unblock) + (void) pthread_sigmask(SIG_UNBLOCK, &new_ss, NULL); + return r; + } /* Use the signal name as description for the event source by default */ (void) sd_event_source_set_description(s, signal_to_string(sig)); diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h index e782339c4a..d2886b8038 100644 --- a/src/systemd/sd-event.h +++ b/src/systemd/sd-event.h @@ -68,6 +68,8 @@ enum { SD_EVENT_PRIORITY_IDLE = 100 }; +#define SD_EVENT_SIGNAL_PROCMASK (1 << 30) + typedef int (*sd_event_handler_t)(sd_event_source *s, void *userdata); typedef int (*sd_event_io_handler_t)(sd_event_source *s, int fd, uint32_t revents, void *userdata); typedef int (*sd_event_time_handler_t)(sd_event_source *s, uint64_t usec, void *userdata); -- 2.25.1