From: Michael Tremer Date: Wed, 3 Aug 2022 15:21:49 +0000 (+0000) Subject: jail: Unify the wait logic for processes and use pidfd X-Git-Tag: 0.9.28~597 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d853213d43ce46e8a7d64c691c04e8927e4cac7b;p=pakfire.git jail: Unify the wait logic for processes and use pidfd Signed-off-by: Michael Tremer --- diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c index 999baf4ac..eee2289ad 100644 --- a/src/libpakfire/jail.c +++ b/src/libpakfire/jail.c @@ -19,9 +19,10 @@ #############################################################################*/ #include -#include #include +#include #include +#include #include #include #include @@ -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))