From: Michael Tremer Date: Thu, 4 Aug 2022 15:18:54 +0000 (+0000) Subject: jail: Implement better logging for the child process X-Git-Tag: 0.9.28~587 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e33387d3c9e708cfa9f22f33c6f0ade6553412cc;p=pakfire.git jail: Implement better logging for the child process The child process used to dump any logging to the standard output/error which is not useful when we want to collect any actual output of the process that was called. This patch adds extra pipes (one for each log level - I know) and passes those log messages on to the regular logger (past the jail log callback). Signed-off-by: Michael Tremer --- diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c index 758443383..2d1778d31 100644 --- a/src/libpakfire/jail.c +++ b/src/libpakfire/jail.c @@ -96,15 +96,25 @@ struct pakfire_jail_exec { int completed_fd; // Log pipes - struct { + struct pakfire_jail_pipes { int stdout[2]; int stderr[2]; + + // Logging + int log_INFO[2]; + int log_ERROR[2]; + int log_DEBUG[2]; } pipes; // Log buffers - struct { + struct pakfire_jail_buffers { struct pakfire_log_buffer stdout; struct pakfire_log_buffer stderr; + + // Logging + struct pakfire_log_buffer log_INFO; + struct pakfire_log_buffer log_ERROR; + struct pakfire_log_buffer log_DEBUG; } buffers; }; @@ -123,6 +133,9 @@ static void pakfire_jail_free(struct pakfire_jail* jail) { free(jail); } +/* + Passes any log messages on to the default pakfire log callback +*/ static int pakfire_jail_default_log_callback(struct pakfire* pakfire, void* data, int priority, const char* line, size_t length) { switch (priority) { @@ -133,6 +146,12 @@ static int pakfire_jail_default_log_callback(struct pakfire* pakfire, void* data case LOG_ERR: ERROR(pakfire, "%s", line); break; + +#ifdef ENABLE_DEBUG + case LOG_DEBUG: + DEBUG(pakfire, "%s", line); + break; +#endif } return 0; @@ -354,6 +373,41 @@ PAKFIRE_EXPORT int pakfire_jail_set_log_callback(struct pakfire_jail* jail, return 0; } +/* + This function replaces any logging in the child process. + + All log messages will be sent to the parent process through their respective pipes. +*/ +static void pakfire_jail_log(void* data, int priority, const char* file, + int line, const char* fn, const char* format, va_list args) { + struct pakfire_jail_pipes* pipes = (struct pakfire_jail_pipes*)data; + int fd; + + switch (priority) { + case LOG_INFO: + fd = pipes->log_INFO[1]; + break; + + case LOG_ERR: + fd = pipes->log_ERROR[1]; + break; + +#ifdef ENABLE_DEBUG + case LOG_DEBUG: + fd = pipes->log_DEBUG[1]; + break; +#endif /* ENABLE_DEBUG */ + + // Ignore any messages of an unknown priority + default: + return; + } + + // Send the log message + if (fd) + vdprintf(fd, format, args); +} + static int pakfire_jail_log_buffer_is_full(const struct pakfire_log_buffer* buffer) { return (sizeof(buffer->data) == buffer->used); } @@ -364,7 +418,8 @@ static int pakfire_jail_log_buffer_is_full(const struct pakfire_log_buffer* buff If not newline character is found, it will try to read more data until it finds one. */ static int pakfire_jail_handle_log(struct pakfire_jail* jail, - struct pakfire_jail_exec* ctx, int priority, int fd, struct pakfire_log_buffer* buffer) { + struct pakfire_jail_exec* ctx, int priority, int fd, + struct pakfire_log_buffer* buffer, pakfire_jail_log_callback callback, void* data) { char line[BUFFER_SIZE + 1]; // Fill up buffer from fd @@ -411,8 +466,8 @@ static int pakfire_jail_handle_log(struct pakfire_jail* jail, line[length] = '\0'; // Log the line - if (jail->log_callback) { - int r = jail->log_callback(jail->pakfire, jail->log_data, priority, line, length); + if (callback) { + int r = callback(jail->pakfire, data, priority, line, length); if (r) { ERROR(jail->pakfire, "The logging callback returned an error: %d\n", r); return r; @@ -427,6 +482,25 @@ static int pakfire_jail_handle_log(struct pakfire_jail* jail, return 0; } +/* + This is a convenience function to fetch the reading end of a pipe and + closes the write end. +*/ +static int pakfire_jail_get_pipe(struct pakfire_jail* jail, int (*fds)[2]) { + // Give the variables easier names to avoid confusion + int* fd_read = &(*fds)[0]; + int* fd_write = &(*fds)[1]; + + // Close the write end of the pipe + if (*fd_write) { + close(*fd_write); + *fd_write = 0; + } + + // Return the read end + return *fd_read; +} + static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec* ctx) { int epollfd = -1; struct epoll_event ev; @@ -434,23 +508,18 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec int r = 0; // Fetch file descriptors from context - const int stdout = ctx->pipes.stdout[0]; - const int stderr = ctx->pipes.stderr[0]; + const int stdout = pakfire_jail_get_pipe(jail, &ctx->pipes.stdout); + const int stderr = pakfire_jail_get_pipe(jail, &ctx->pipes.stderr); const int pidfd = ctx->pidfd; - // Close any unused file descriptors - if (ctx->pipes.stdout[1]) { - close(ctx->pipes.stdout[1]); - ctx->pipes.stdout[1] = 0; - } - if (ctx->pipes.stderr[1]) { - close(ctx->pipes.stderr[1]); - ctx->pipes.stderr[1] = 0; - } + // Logging + const int log_INFO = pakfire_jail_get_pipe(jail, &ctx->pipes.log_INFO); + const int log_ERROR = pakfire_jail_get_pipe(jail, &ctx->pipes.log_ERROR); + const int log_DEBUG = pakfire_jail_get_pipe(jail, &ctx->pipes.log_DEBUG); // Make a list of all file descriptors we are interested in int fds[] = { - stdout, stderr, pidfd, + stdout, stderr, pidfd, log_INFO, log_ERROR, log_DEBUG, }; // Setup epoll @@ -496,12 +565,14 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec goto ERROR; } - struct pakfire_log_buffer* buffer; - int priority; - for (int i = 0; i < num; i++) { int fd = events[i].data.fd; + struct pakfire_log_buffer* buffer = NULL; + pakfire_jail_log_callback callback = NULL; + void* data = NULL; + int priority; + // Handle any changes to the PIDFD if (fd == pidfd) { // Call waidid() and store the result @@ -516,22 +587,47 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec ended = 1; continue; + // Handle logging messages + } else if (fd == log_INFO) { + buffer = &ctx->buffers.log_INFO; + priority = LOG_INFO; + + callback = pakfire_jail_default_log_callback; + + } else if (fd == log_ERROR) { + buffer = &ctx->buffers.log_ERROR; + priority = LOG_ERR; + + callback = pakfire_jail_default_log_callback; + + } else if (fd == log_DEBUG) { + buffer = &ctx->buffers.log_DEBUG; + priority = LOG_DEBUG; + + callback = pakfire_jail_default_log_callback; + // Handle anything from the log pipes } else if (fd == stdout) { buffer = &ctx->buffers.stdout; priority = LOG_INFO; + callback = jail->log_callback; + data = jail->log_data; + } else if (fd == stderr) { buffer = &ctx->buffers.stderr; priority = LOG_ERR; + callback = jail->log_callback; + data = jail->log_data; + } else { DEBUG(jail->pakfire, "Received invalid file descriptor %d\n", fd); continue; } // Handle log event - r = pakfire_jail_handle_log(jail, ctx, priority, fd, buffer); + r = pakfire_jail_handle_log(jail, ctx, priority, fd, buffer, callback, data); if (r) goto ERROR; } @@ -955,7 +1051,8 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe const char* argv[]) { int r; - // XXX do we have to reconfigure logging here? + // Redirect any logging to our log pipe + pakfire_set_log_callback(jail->pakfire, pakfire_jail_log, &ctx->pipes); // Fetch my own PID pid_t pid = getpid(); @@ -1035,6 +1132,13 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe } } + // Close other end of log pipes + close(ctx->pipes.log_INFO[0]); + close(ctx->pipes.log_ERROR[0]); +#ifdef ENABLE_DEBUG + close(ctx->pipes.log_DEBUG[0]); +#endif /* ENABLE_DEBUG */ + // Connect standard output and error if (ctx->pipes.stdout[1] && ctx->pipes.stderr[1]) { r = dup2(ctx->pipes.stdout[1], STDOUT_FILENO); @@ -1092,6 +1196,22 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe return r; } +static int pakfire_jail_setup_pipe(struct pakfire_jail* jail, int (*fds)[2], const int flags) { + int r = pipe2(*fds, flags); + if (r < 0) { + ERROR(jail->pakfire, "Could not setup pipe: %m\n"); + return 1; + } + + return 0; +} + +static void pakfire_jail_close_pipe(struct pakfire_jail* jail, int fds[2]) { + for (unsigned int i = 0; i < 2; i++) + if (fds[i]) + close(fds[i]); +} + // Run a command in the jail static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) { int exit = -1; @@ -1106,8 +1226,8 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) { // Initialize context for this call struct pakfire_jail_exec ctx = { .pipes = { - .stdout = { 0, 0, }, - .stderr = { 0, 0, }, + .stdout = { 0, 0 }, + .stderr = { 0, 0 }, }, }; @@ -1126,20 +1246,34 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) { // Create pipes to communicate with child process if we are not running interactively if (!pakfire_jail_has_flag(jail, PAKFIRE_JAIL_INTERACTIVE)) { // stdout - r = pipe(ctx.pipes.stdout); - if (r < 0) { - ERROR(jail->pakfire, "Could not create file descriptors for stdout: %m\n"); + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.stdout, 0); + if (r) goto ERROR; - } // stderr - r = pipe(ctx.pipes.stderr); - if (r < 0) { - ERROR(jail->pakfire, "Could not create file descriptors for stderr: %m\n"); + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.stderr, 0); + if (r) goto ERROR; - } } + // Setup pipes for logging + // INFO + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.log_INFO, O_CLOEXEC); + if (r) + goto ERROR; + + // ERROR + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.log_ERROR, O_CLOEXEC); + if (r) + goto ERROR; + +#ifdef ENABLE_DEBUG + // DEBUG + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.log_DEBUG, O_CLOEXEC); + if (r) + goto ERROR; +#endif /* ENABLE_DEBUG */ + // Configure child process struct clone_args args = { .flags = @@ -1201,16 +1335,13 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[]) { ERROR: // Close any file descriptors - if (ctx.pipes.stdout[0]) - close(ctx.pipes.stdout[0]); - if (ctx.pipes.stdout[1]) - close(ctx.pipes.stdout[1]); - if (ctx.pipes.stderr[0]) - close(ctx.pipes.stderr[0]); - if (ctx.pipes.stderr[1]) - close(ctx.pipes.stderr[1]); + pakfire_jail_close_pipe(jail, ctx.pipes.stdout); + pakfire_jail_close_pipe(jail, ctx.pipes.stderr); if (ctx.pidfd) close(ctx.pidfd); + pakfire_jail_close_pipe(jail, ctx.pipes.log_INFO); + pakfire_jail_close_pipe(jail, ctx.pipes.log_ERROR); + pakfire_jail_close_pipe(jail, ctx.pipes.log_DEBUG); // Umount everything if (!pakfire_on_root(jail->pakfire))