From a75211421fc9366068e6d9446e8e567246c72feb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sun, 6 Jan 2019 22:17:00 +0100
Subject: [PATCH] udev: rework how we handle the return value from spawned
 programs

When running PROGRAM="...", we would log
systemd-udevd[447]: Failed to wait spawned command '...': Input/output error
no matter why the program actually failed, at error level.

The code wouldn't distinguish between an internal failure and a failure in the
program being called and run sd_event_exit(..., -EIO) on any kind of error. EIO
is rather misleading here, becuase it suggests a serious error.

on_spawn_sigchld is updated to set the return code to distinguish failure to
spawn, including the program being killed by a signal (a negative return value),
and the program failing (positive return value).

The logging levels are adjusted, so that for PROGRAM= calls, which are
essentially "if" statements, we only log at debug level (unless we get a
timeout or segfault or another unexpected error).
---
 src/udev/udev-event.c | 38 +++++++++++++-------------------------
 src/udev/udev-rules.c | 12 +++++++-----
 2 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 3e916976c0..07b7365e3a 100644
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -504,38 +504,34 @@ static int on_spawn_timeout_warning(sd_event_source *s, uint64_t usec, void *use
 
 static int on_spawn_sigchld(sd_event_source *s, const siginfo_t *si, void *userdata) {
         Spawn *spawn = userdata;
+        int ret = -EIO;
 
         assert(spawn);
 
         switch (si->si_code) {
         case CLD_EXITED:
-                if (si->si_status == 0) {
+                if (si->si_status == 0)
                         log_debug("Process '%s' succeeded.", spawn->cmd);
-                        sd_event_exit(sd_event_source_get_event(s), 0);
-
-                        return 1;
-                }
-
-                log_full(spawn->accept_failure ? LOG_DEBUG : LOG_WARNING,
-                         "Process '%s' failed with exit code %i.", spawn->cmd, si->si_status);
+                else
+                        log_full(spawn->accept_failure ? LOG_DEBUG : LOG_WARNING,
+                                 "Process '%s' failed with exit code %i.", spawn->cmd, si->si_status);
+                ret = si->si_status;
                 break;
         case CLD_KILLED:
         case CLD_DUMPED:
-                log_warning("Process '%s' terminated by signal %s.", spawn->cmd, signal_to_string(si->si_status));
-
+                log_error("Process '%s' terminated by signal %s.", spawn->cmd, signal_to_string(si->si_status));
                 break;
         default:
                 log_error("Process '%s' failed due to unknown reason.", spawn->cmd);
         }
 
-        sd_event_exit(sd_event_source_get_event(s), -EIO);
-
+        sd_event_exit(sd_event_source_get_event(s), ret);
         return 1;
 }
 
 static int spawn_wait(Spawn *spawn) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
-        int r, ret;
+        int r;
 
         assert(spawn);
 
@@ -586,15 +582,7 @@ static int spawn_wait(Spawn *spawn) {
         if (r < 0)
                 return r;
 
-        r = sd_event_loop(e);
-        if (r < 0)
-                return r;
-
-        r = sd_event_get_exit_code(e, &ret);
-        if (r < 0)
-                return r;
-
-        return ret;
+        return sd_event_loop(e);
 }
 
 int udev_event_spawn(UdevEvent *event,
@@ -679,12 +667,12 @@ int udev_event_spawn(UdevEvent *event,
         };
         r = spawn_wait(&spawn);
         if (r < 0)
-                return log_error_errno(r, "Failed to wait spawned command '%s': %m", cmd);
+                return log_error_errno(r, "Failed to wait for spawned command '%s': %m", cmd);
 
         if (result)
                 result[spawn.result_len] = '\0';
 
-        return r;
+        return r; /* 0 for success, and positive if the program failed */
 }
 
 static int rename_netif(UdevEvent *event) {
@@ -899,7 +887,7 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec) {
                                 (void) usleep(event->exec_delay_usec);
                         }
 
-                        udev_event_spawn(event, timeout_usec, false, command, NULL, 0);
+                        (void) udev_event_spawn(event, timeout_usec, false, command, NULL, 0);
                 }
         }
 }
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 53c68d254a..f697972deb 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -645,11 +645,13 @@ static int import_program_into_properties(UdevEvent *event,
                                           const char *program) {
         char result[UTIL_LINE_SIZE];
         char *line;
-        int err;
+        int r;
 
-        err = udev_event_spawn(event, timeout_usec, true, program, result, sizeof(result));
-        if (err < 0)
-                return err;
+        r = udev_event_spawn(event, timeout_usec, false, program, result, sizeof result);
+        if (r < 0)
+                return r;
+        if (r > 0)
+                return -EIO;
 
         line = result;
         while (line) {
@@ -1959,7 +1961,7 @@ int udev_rules_apply_to_event(
                                          rules_str(rules, rule->rule.filename_off),
                                          rule->rule.filename_line);
 
-                        if (udev_event_spawn(event, timeout_usec, true, program, result, sizeof(result)) < 0) {
+                        if (udev_event_spawn(event, timeout_usec, true, program, result, sizeof(result)) != 0) {
                                 if (cur->key.op != OP_NOMATCH)
                                         goto nomatch;
                         } else {