udevd: simplify signal mask handling
authorTom Gundersen <teg@jklm.no>
Tue, 2 Jun 2015 15:07:21 +0000 (17:07 +0200)
committerTom Gundersen <teg@jklm.no>
Tue, 2 Jun 2015 23:41:34 +0000 (01:41 +0200)
We used to block all signals, and restore the original signal mask before exec'ing
external processes.

Now we just block the signals we care about and unconditionally unblock all signals
before exec'ing.

src/test/test-udev.c
src/udev/udev-event.c
src/udev/udev-rules.c
src/udev/udev.h
src/udev/udevadm-test.c
src/udev/udevd.c

index f3953fe..d1fe953 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "missing.h"
 #include "selinux-util.h"
+#include "signal-util.h"
 #include "udev.h"
 #include "udev-util.h"
 
@@ -79,7 +80,6 @@ int main(int argc, char *argv[]) {
         char syspath[UTIL_PATH_SIZE];
         const char *devpath;
         const char *action;
-        sigset_t mask, sigmask_orig;
         int err;
 
         err = fake_filesystems();
@@ -93,8 +93,6 @@ int main(int argc, char *argv[]) {
         log_debug("version %s", VERSION);
         mac_selinux_init("/dev");
 
-        sigprocmask(SIG_SETMASK, NULL, &sigmask_orig);
-
         action = argv[1];
         if (action == NULL) {
                 log_error("action missing");
@@ -118,8 +116,7 @@ int main(int argc, char *argv[]) {
 
         event = udev_event_new(dev);
 
-        sigfillset(&mask);
-        sigprocmask(SIG_SETMASK, &mask, &sigmask_orig);
+        assert_se(sigprocmask_many(SIG_BLOCK, SIGTERM, SIGINT, SIGHUP, SIGCHLD, -1) == 0);
 
         /* do what devtmpfs usually provides us */
         if (udev_device_get_devnode(dev) != NULL) {
@@ -142,11 +139,9 @@ int main(int argc, char *argv[]) {
         udev_event_execute_rules(event,
                                  3 * USEC_PER_SEC, USEC_PER_SEC,
                                  NULL,
-                                 rules,
-                                 &sigmask_orig);
+                                 rules);
         udev_event_execute_run(event,
-                               3 * USEC_PER_SEC, USEC_PER_SEC,
-                               NULL);
+                               3 * USEC_PER_SEC, USEC_PER_SEC);
 out:
         mac_selinux_finish();
 
index 3488ff3..4dcf8f2 100644 (file)
@@ -385,7 +385,7 @@ out:
 }
 
 static int spawn_exec(struct udev_event *event,
-                      const char *cmd, char *const argv[], char **envp, const sigset_t *sigmask,
+                      const char *cmd, char *const argv[], char **envp,
                       int fd_stdout, int fd_stderr) {
         _cleanup_close_ int fd = -1;
 
@@ -413,9 +413,8 @@ static int spawn_exec(struct udev_event *event,
         /* terminate child in case parent goes away */
         prctl(PR_SET_PDEATHSIG, SIGTERM);
 
-        /* restore original udev sigmask before exec */
-        if (sigmask)
-                sigprocmask(SIG_SETMASK, sigmask, NULL);
+        /* restore sigmask before exec */
+        (void) reset_signal_mask();
 
         execve(argv[0], argv, envp);
 
@@ -699,7 +698,7 @@ out:
 int udev_event_spawn(struct udev_event *event,
                      usec_t timeout_usec,
                      usec_t timeout_warn_usec,
-                     const char *cmd, char **envp, const sigset_t *sigmask,
+                     const char *cmd, char **envp,
                      char *result, size_t ressize) {
         int outpipe[2] = {-1, -1};
         int errpipe[2] = {-1, -1};
@@ -749,7 +748,7 @@ int udev_event_spawn(struct udev_event *event,
 
                 log_debug("starting '%s'", cmd);
 
-                spawn_exec(event, cmd, argv, envp, sigmask,
+                spawn_exec(event, cmd, argv, envp,
                            outpipe[WRITE_END], errpipe[WRITE_END]);
 
                 _exit(2 );
@@ -811,8 +810,7 @@ static int rename_netif(struct udev_event *event) {
 void udev_event_execute_rules(struct udev_event *event,
                               usec_t timeout_usec, usec_t timeout_warn_usec,
                               struct udev_list *properties_list,
-                              struct udev_rules *rules,
-                              const sigset_t *sigmask) {
+                              struct udev_rules *rules) {
         struct udev_device *dev = event->dev;
 
         if (udev_device_get_subsystem(dev) == NULL)
@@ -828,8 +826,7 @@ void udev_event_execute_rules(struct udev_event *event,
 
                 udev_rules_apply_to_event(rules, event,
                                           timeout_usec, timeout_warn_usec,
-                                          properties_list,
-                                          sigmask);
+                                          properties_list);
 
                 if (major(udev_device_get_devnum(dev)) != 0)
                         udev_node_remove(dev);
@@ -847,8 +844,7 @@ void udev_event_execute_rules(struct udev_event *event,
 
                 udev_rules_apply_to_event(rules, event,
                                           timeout_usec, timeout_warn_usec,
-                                          properties_list,
-                                          sigmask);
+                                          properties_list);
 
                 /* rename a new network interface, if needed */
                 if (udev_device_get_ifindex(dev) > 0 && streq(udev_device_get_action(dev), "add") &&
@@ -911,7 +907,7 @@ void udev_event_execute_rules(struct udev_event *event,
         }
 }
 
-void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec, const sigset_t *sigmask) {
+void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec) {
         struct udev_list_entry *list_entry;
 
         udev_list_entry_foreach(list_entry, udev_list_get_entry(&event->run_list)) {
@@ -934,7 +930,7 @@ void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_
 
                         udev_event_apply_format(event, cmd, program, sizeof(program));
                         envp = udev_device_get_properties_envp(event->dev);
-                        udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, sigmask, NULL, 0);
+                        udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, NULL, 0);
                 }
         }
 }
index c2b20cf..9153715 100644 (file)
@@ -633,7 +633,7 @@ static int import_file_into_properties(struct udev_device *dev, const char *file
 static int import_program_into_properties(struct udev_event *event,
                                           usec_t timeout_usec,
                                           usec_t timeout_warn_usec,
-                                          const char *program, const sigset_t *sigmask) {
+                                          const char *program) {
         struct udev_device *dev = event->dev;
         char **envp;
         char result[UTIL_LINE_SIZE];
@@ -641,7 +641,7 @@ static int import_program_into_properties(struct udev_event *event,
         int err;
 
         envp = udev_device_get_properties_envp(dev);
-        err = udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, sigmask, result, sizeof(result));
+        err = udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, result, sizeof(result));
         if (err < 0)
                 return err;
 
@@ -1895,8 +1895,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                               struct udev_event *event,
                               usec_t timeout_usec,
                               usec_t timeout_warn_usec,
-                              struct udev_list *properties_list,
-                              const sigset_t *sigmask) {
+                              struct udev_list *properties_list) {
         struct token *cur;
         struct token *rule;
         enum escape_type esc = ESCAPE_UNSET;
@@ -2132,7 +2131,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                                   rules_str(rules, rule->rule.filename_off),
                                   rule->rule.filename_line);
 
-                        if (udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, sigmask, result, sizeof(result)) < 0) {
+                        if (udev_event_spawn(event, timeout_usec, timeout_warn_usec, program, envp, result, sizeof(result)) < 0) {
                                 if (cur->key.op != OP_NOMATCH)
                                         goto nomatch;
                         } else {
@@ -2168,7 +2167,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules,
                                   rules_str(rules, rule->rule.filename_off),
                                   rule->rule.filename_line);
 
-                        if (import_program_into_properties(event, timeout_usec, timeout_warn_usec, import, sigmask) != 0)
+                        if (import_program_into_properties(event, timeout_usec, timeout_warn_usec, import) != 0)
                                 if (cur->key.op != OP_NOMATCH)
                                         goto nomatch;
                         break;
index 1b17c61..fd8504c 100644 (file)
@@ -20,7 +20,6 @@
 
 #include <sys/types.h>
 #include <sys/param.h>
-#include <signal.h>
 
 #include "macro.h"
 #include "sd-rtnl.h"
@@ -73,8 +72,7 @@ struct udev_rules *udev_rules_unref(struct udev_rules *rules);
 bool udev_rules_check_timestamp(struct udev_rules *rules);
 int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event,
                               usec_t timeout_usec, usec_t timeout_warn_usec,
-                              struct udev_list *properties_list,
-                              const sigset_t *sigmask);
+                              struct udev_list *properties_list);
 int udev_rules_apply_static_dev_perms(struct udev_rules *rules);
 
 /* udev-event.c */
@@ -86,14 +84,13 @@ int udev_event_apply_subsys_kernel(struct udev_event *event, const char *string,
 int udev_event_spawn(struct udev_event *event,
                      usec_t timeout_usec,
                      usec_t timeout_warn_usec,
-                     const char *cmd, char **envp, const sigset_t *sigmask,
+                     const char *cmd, char **envp,
                      char *result, size_t ressize);
 void udev_event_execute_rules(struct udev_event *event,
                               usec_t timeout_usec, usec_t timeout_warn_usec,
                               struct udev_list *properties_list,
-                              struct udev_rules *rules,
-                              const sigset_t *sigset);
-void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec, const sigset_t *sigset);
+                              struct udev_rules *rules);
+void udev_event_execute_run(struct udev_event *event, usec_t timeout_usec, usec_t timeout_warn_usec);
 int udev_build_argv(struct udev *udev, char *cmd, int *argc, char *argv[]);
 
 /* udev-watch.c */
index 46ec0e3..d04e618 100644 (file)
@@ -135,8 +135,7 @@ static int adm_test(struct udev *udev, int argc, char *argv[]) {
         udev_event_execute_rules(event,
                                  60 * USEC_PER_SEC, 20 * USEC_PER_SEC,
                                  NULL,
-                                 rules,
-                                 &sigmask_orig);
+                                 rules);
 
         udev_list_entry_foreach(entry, udev_device_get_properties_list_entry(dev))
                 printf("%s=%s\n", udev_list_entry_get_name(entry), udev_list_entry_get_value(entry));
index 056cf8c..830bad9 100644 (file)
@@ -42,6 +42,8 @@
 
 #include "sd-daemon.h"
 #include "sd-event.h"
+
+#include "signal-util.h"
 #include "event-util.h"
 #include "rtnl-util.h"
 #include "cgroup-util.h"
@@ -69,7 +71,6 @@ typedef struct Manager {
         struct udev_list_node events;
         char *cgroup;
         pid_t pid; /* the process that originally allocated the manager object */
-        sigset_t sigmask_orig;
 
         struct udev_rules *rules;
         struct udev_list properties;
@@ -448,12 +449,10 @@ static void worker_spawn(Manager *manager, struct event *event) {
                         udev_event_execute_rules(udev_event,
                                                  arg_event_timeout_usec, arg_event_timeout_warn_usec,
                                                  &manager->properties,
-                                                 manager->rules,
-                                                 &manager->sigmask_orig);
+                                                 manager->rules);
 
                         udev_event_execute_run(udev_event,
-                                               arg_event_timeout_usec, arg_event_timeout_warn_usec,
-                                               &manager->sigmask_orig);
+                                               arg_event_timeout_usec, arg_event_timeout_warn_usec);
 
                         if (udev_event->rtnl)
                                 /* in case rtnl was initialized */
@@ -1513,7 +1512,6 @@ static int manager_new(Manager **ret) {
 }
 
 static int manager_listen(Manager *manager) {
-        sigset_t mask;
         int r, fd_worker, one = 1;
 
         assert(manager);
@@ -1536,8 +1534,7 @@ static int manager_listen(Manager *manager) {
         udev_watch_restore(manager->udev);
 
         /* block and listen to all signals on signalfd */
-        sigfillset(&mask);
-        sigprocmask(SIG_SETMASK, &mask, &manager->sigmask_orig);
+        assert_se(sigprocmask_many(SIG_BLOCK, SIGTERM, SIGINT, SIGHUP, SIGCHLD, -1) == 0);
 
         r = sd_event_default(&manager->event);
         if (r < 0)