jail: Unify the wait logic for processes and use pidfd

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>

diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c
index 999baf4ac1ad1777bc317ae43c61bd606c4cd223..eee2289ad288900a972fc8ecf99ef2e1d16320d4 100644 (file)
--- a/src/libpakfire/jail.c
+++ b/src/libpakfire/jail.c
@@ -19,9 +19,10 @@
 #############################################################################*/
 
 #include <errno.h>
-#include <fcntl.h>
 #include <linux/capability.h>
+#include <linux/fcntl.h>
 #include <linux/sched.h>
+#include <linux/wait.h>
 #include <sched.h>
 #include <signal.h>
 #include <stdlib.h>
@@ -82,9 +83,10 @@ struct pakfire_log_buffer {
 struct pakfire_jail_exec {
        // PID (of the child)
        pid_t pid;
+       int pidfd;
 
-       // Process status (from waitpid)
-       int status;
+       // Process status (from waitid)
+       siginfo_t status;
 
        // FD to notify the client that the parent has finished initialization
        int completed_fd;
@@ -406,7 +408,7 @@ static int pakfire_jail_handle_log(struct pakfire_jail* jail,
        return 0;
 }
 
-static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_exec* ctx) {
+static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec* ctx) {
        int epollfd = -1;
        struct epoll_event ev;
        struct epoll_event events[EPOLL_MAX_EVENTS];
@@ -415,9 +417,10 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
        // Fetch file descriptors from context
        const int stdout = ctx->pipes.stdout[1];
        const int stderr = ctx->pipes.stderr[1];
+       const int pidfd  = ctx->pidfd;
 
-       int fds[2] = {
-               stdout, stderr,
+       int fds[] = {
+               stdout, stderr, pidfd,
        };
 
        // Setup epoll
@@ -425,21 +428,25 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
        if (epollfd < 0) {
                ERROR(jail->pakfire, "Could not initialize epoll(): %m\n");
                r = 1;
-               goto OUT;
+               goto ERROR;
        }
 
        ev.events = EPOLLIN;
 
        // Turn file descriptors into non-blocking mode and add them to epoll()
-       for (unsigned int i = 0; i < 2; i++) {
+       for (unsigned int i = 0; i < 3; i++) {
                int fd = fds[i];
 
+               // Skip fds which were not initialized
+               if (fd <= 0)
+                       continue;
+
                ev.data.fd = fd;
 
                if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) < 0) {
                        ERROR(jail->pakfire, "Could not add file descriptor %d to epoll(): %m\n", fd);
                        r = 1;
-                       goto OUT;
+                       goto ERROR;
                }
        }
 
@@ -447,13 +454,6 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
 
        // Loop for as long as the process is alive
        while (!ended) {
-               // If waitpid() returns non-zero, the process has ended, but we want to perform
-               // one last iteration over the loop to read any remaining content from the file
-               // descriptor buffers.
-               r = waitpid(ctx->pid, &ctx->status, WNOHANG);
-               if (r)
-                       ended = 1;
-
                int num = epoll_wait(epollfd, events, EPOLL_MAX_EVENTS, -1);
                if (num < 1) {
                        // Ignore if epoll_wait() has been interrupted
@@ -463,7 +463,7 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
                        ERROR(jail->pakfire, "epoll_wait() failed: %m\n");
                        r = 1;
 
-                       goto OUT;
+                       goto ERROR;
                }
 
                struct pakfire_log_buffer* buffer;
@@ -472,7 +472,22 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
                for (int i = 0; i < num; i++) {
                        int fd = events[i].data.fd;
 
-                       if (fd == stdout) {
+                       // Handle any changes to the PIDFD
+                       if (fd == pidfd) {
+                               // Call waidid() and store the result
+                               r = waitid(P_PIDFD, ctx->pidfd, &ctx->status, WEXITED);
+                               if (r) {
+                                       ERROR(jail->pakfire, "waitid() failed: %m\n");
+                                       goto ERROR;
+                               }
+
+                               // Mark that we have ended so that we will process the remaining
+                               // events from epoll() now, but won't restart the outer loop.
+                               ended = 1;
+                               continue;
+
+                       // Handle anything from the log pipes
+                       } else if (fd == stdout) {
                                buffer = &ctx->buffers.stdout;
                                priority = LOG_INFO;
 
@@ -488,11 +503,11 @@ static int pakfire_jail_logger(struct pakfire_jail* jail, struct pakfire_jail_ex
                        // Handle log event
                        r = pakfire_jail_handle_log(jail, ctx, priority, fd, buffer);
                        if (r)
-                               goto OUT;
+                               goto ERROR;
                }
        }
 
-OUT:
+ERROR:
        if (epollfd > 0)
                close(epollfd);
 
@@ -1030,7 +1045,6 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) {
                        .stdout = { 0, 0, },
                        .stderr = { 0, 0, },
                },
-               .status = 0,
        };
 
        DEBUG(jail->pakfire, "Executing jail...\n");
@@ -1070,8 +1084,10 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) {
                        CLONE_NEWNS |
                        CLONE_NEWPID |
                        CLONE_NEWUSER |
-                       CLONE_NEWUTS,
+                       CLONE_NEWUTS |
+                       CLONE_PIDFD,
                .exit_signal = SIGCHLD,
+               .pidfd = (long long unsigned int)&ctx.pidfd,
        };
 
        // Fork this process
@@ -1094,24 +1110,29 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) {
        DEBUG(jail->pakfire, "Waiting for PID %d to finish its work\n", ctx.pid);
 
        // Read output of the child process
-       if (!pakfire_jail_has_flag(jail, PAKFIRE_JAIL_INTERACTIVE)) {
-               r = pakfire_jail_logger(jail, &ctx);
-               if (r)
-                       ERROR(jail->pakfire, "Log reading aborted: %m\n");
-       }
+       r = pakfire_jail_wait(jail, &ctx);
+       if (r)
+               goto ERROR;
 
-       if (!ctx.status)
-               waitpid(ctx.pid, &ctx.status, 0);
+       // Handle exit status
+       switch (ctx.status.si_code) {
+               case CLD_EXITED:
+                       DEBUG(jail->pakfire, "The child process exited with code %d\n",
+                               ctx.status.si_status);
 
-       if (WIFEXITED(ctx.status)) {
-               exit = WEXITSTATUS(ctx.status);
+                       // Pass exit code
+                       exit = ctx.status.si_status;
+                       break;
 
-               DEBUG(jail->pakfire, "Child process exited with code: %d\n", exit);
-       } else {
-               ERROR(jail->pakfire, "Could not determine the exit status of process %d\n", ctx.pid);
+               case CLD_KILLED:
+               case CLD_DUMPED:
+                       ERROR(jail->pakfire, "The child process was killed\n");
+                       break;
 
-               errno = ESRCH;
-               exit = -1;
+               // Log anything else
+               default:
+                       ERROR(jail->pakfire, "Unknown child exit code: %d\n", ctx.status.si_code);
+                       break;
        }
 
 ERROR:
@@ -1124,6 +1145,8 @@ ERROR:
                close(ctx.pipes.stderr[0]);
        if (ctx.pipes.stderr[1])
                close(ctx.pipes.stderr[1]);
+       if (ctx.pidfd)
+               close(ctx.pidfd);
 
        // Umount everything
        if (!pakfire_on_root(jail->pakfire))