]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
shutdown: propagate mount() failures from child to parent
authorLennart Poettering <lennart@poettering.net>
Mon, 9 Jan 2023 13:07:07 +0000 (14:07 +0100)
committerLennart Poettering <lennart@poettering.net>
Mon, 9 Jan 2023 21:17:04 +0000 (22:17 +0100)
Let's propagate the actual error code up, as we usual do.

Inspired by: #25168

src/basic/process-util.c
src/basic/process-util.h
src/shutdown/umount.c

index 009f3f531765fd5371487fca62a413b5ef04b6bf..fa00c962ab55c7434199ff7cf358288528c5b9a3 100644 (file)
@@ -765,6 +765,23 @@ void sigterm_wait(pid_t pid) {
         (void) wait_for_terminate(pid, NULL);
 }
 
+void sigkill_nowait(pid_t pid) {
+        assert(pid > 1);
+
+        (void) kill(pid, SIGKILL);
+}
+
+void sigkill_nowaitp(pid_t *pid) {
+        PROTECT_ERRNO;
+
+        if (!pid)
+                return;
+        if (*pid <= 1)
+                return;
+
+        sigkill_nowait(*pid);
+}
+
 int kill_and_sigcont(pid_t pid, int sig) {
         int r;
 
index 8f265e703d44e54f0fa6ae430cdf45110978a46e..8a981cd7301cbd6c5e4244070ba7efb5d90cebc4 100644 (file)
@@ -68,6 +68,8 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout);
 void sigkill_wait(pid_t pid);
 void sigkill_waitp(pid_t *pid);
 void sigterm_wait(pid_t pid);
+void sigkill_nowait(pid_t pid);
+void sigkill_nowaitp(pid_t *pid);
 
 int kill_and_sigcont(pid_t pid, int sig);
 
index 3ccd7173e3ccf13efe3296d86c069a5c954f15b8..13a80b8e7f069e30cbe44c56fdf310eb188dd789 100644 (file)
@@ -594,19 +594,26 @@ static void log_umount_blockers(const char *mnt) {
 }
 
 static int remount_with_timeout(MountPoint *m, bool last_try) {
-        pid_t pid;
+        _cleanup_(close_pairp) int pfd[2] = PIPE_EBADF;
+        _cleanup_(sigkill_nowaitp) pid_t pid = 0;
         int r;
 
         BLOCK_SIGNALS(SIGCHLD);
 
         assert(m);
 
+        r = pipe2(pfd, O_CLOEXEC|O_NONBLOCK);
+        if (r < 0)
+                return r;
+
         /* Due to the possibility of a remount operation hanging, we fork a child process and set a
          * timeout. If the timeout lapses, the assumption is that the particular remount failed. */
         r = safe_fork("(sd-remount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pid);
         if (r < 0)
                 return r;
         if (r == 0) {
+                pfd[0] = safe_close(pfd[0]);
+
                 log_info("Remounting '%s' read-only with options '%s'.", m->path, strempty(m->remount_options));
 
                 /* Start the mount operation here in the child */
@@ -617,35 +624,49 @@ static int remount_with_timeout(MountPoint *m, bool last_try) {
                                        "Failed to remount '%s' read-only: %m",
                                        m->path);
 
+                (void) write(pfd[1], &r, sizeof(r)); /* try to send errno up */
                 _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
 
+        pfd[1] = safe_close(pfd[1]);
+
         r = wait_for_terminate_with_timeout(pid, DEFAULT_TIMEOUT_USEC);
-        if (r == -ETIMEDOUT) {
+        if (r == -ETIMEDOUT)
                 log_error_errno(r, "Remounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pid);
-                (void) kill(pid, SIGKILL);
-        } else if (r == -EPROTO)
-                log_debug_errno(r, "Remounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
-        else if (r < 0)
+        else if (r == -EPROTO) {
+                /* Try to read error code from child */
+                if (read(pfd[0], &r, sizeof(r)) == sizeof(r))
+                        log_debug_errno(r, "Remounting '%s' failed abnormally, child process " PID_FMT " failed: %m", m->path, pid);
+                else
+                        log_debug_errno(r, "Remounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
+                TAKE_PID(pid); /* child exited (just not as we expected) hence don't kill anymore */
+        } else if (r < 0)
                 log_error_errno(r, "Remounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pid);
 
         return r;
 }
 
 static int umount_with_timeout(MountPoint *m, bool last_try) {
-        pid_t pid;
+        _cleanup_(close_pairp) int pfd[2] = PIPE_EBADF;
+        _cleanup_(sigkill_nowaitp) pid_t pid = 0;
         int r;
 
         BLOCK_SIGNALS(SIGCHLD);
 
         assert(m);
 
+        r = pipe2(pfd, O_CLOEXEC|O_NONBLOCK);
+        if (r < 0)
+                return r;
+
         /* Due to the possibility of a umount operation hanging, we fork a child process and set a
          * timeout. If the timeout lapses, the assumption is that the particular umount failed. */
         r = safe_fork("(sd-umount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_REOPEN_LOG, &pid);
         if (r < 0)
                 return r;
         if (r == 0) {
+                pfd[0] = safe_close(pfd[0]);
+
                 log_info("Unmounting '%s'.", m->path);
 
                 /* Start the mount operation here in the child Using MNT_FORCE causes some filesystems
@@ -663,16 +684,23 @@ static int umount_with_timeout(MountPoint *m, bool last_try) {
                                 log_umount_blockers(m->path);
                 }
 
+                (void) write(pfd[1], &r, sizeof(r)); /* try to send errno up */
                 _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
 
+        pfd[1] = safe_close(pfd[1]);
+
         r = wait_for_terminate_with_timeout(pid, DEFAULT_TIMEOUT_USEC);
-        if (r == -ETIMEDOUT) {
+        if (r == -ETIMEDOUT)
                 log_error_errno(r, "Unmounting '%s' timed out, issuing SIGKILL to PID " PID_FMT ".", m->path, pid);
-                (void) kill(pid, SIGKILL);
-        } else if (r == -EPROTO)
-                log_debug_errno(r, "Unmounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
-        else if (r < 0)
+        else if (r == -EPROTO) {
+                /* Try to read error code from child */
+                if (read(pfd[0], &r, sizeof(r)) == sizeof(r))
+                        log_debug_errno(r, "Unmounting '%s' failed abnormally, child process " PID_FMT " failed: %m", m->path, pid);
+                else
+                        r = log_debug_errno(EPROTO, "Unmounting '%s' failed abnormally, child process " PID_FMT " aborted or exited non-zero.", m->path, pid);
+                TAKE_PID(pid); /* It died, but abnormally, no purpose in killing */
+        } else if (r < 0)
                 log_error_errno(r, "Unmounting '%s' failed unexpectedly, couldn't wait for child process " PID_FMT ": %m", m->path, pid);
 
         return r;