From 2015cb9264c6989f9b16d436c30cc21a6f53ac04 Mon Sep 17 00:00:00 2001 From: Michael Tremer Date: Tue, 6 Dec 2022 15:54:54 +0000 Subject: [PATCH] jail: Add new way to communicate with child processes In order to read and write to the child process, a new interface is being added. Signed-off-by: Michael Tremer --- src/_pakfire/pakfire.c | 61 ++++------- src/libpakfire/include/pakfire/jail.h | 20 ++-- src/libpakfire/jail.c | 149 ++++++++++++++++++-------- src/libpakfire/libpakfire.sym | 2 +- tests/libpakfire/jail.c | 41 +++++++ 5 files changed, 178 insertions(+), 95 deletions(-) diff --git a/src/_pakfire/pakfire.c b/src/_pakfire/pakfire.c index 3843d9406..ea25f3ed2 100644 --- a/src/_pakfire/pakfire.c +++ b/src/_pakfire/pakfire.c @@ -774,7 +774,7 @@ static PyObject* Pakfire_version_compare(PakfireObject* self, PyObject* args) { return PyLong_FromLong(cmp); } -static int __Pakfire_logging_callback(struct pakfire* pakfire, void* data, +static int Pakfire_execute_output_callback(struct pakfire* pakfire, void* data, int priority, const char* line, size_t length) { PyObject* callback = (PyObject*)data; int r = 0; @@ -819,9 +819,8 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* "command", "environ", "bind", - "logging_callback", + "callback", "nice", - "return_output", NULL }; @@ -830,17 +829,15 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* int flags = 0; int r; PyObject* ret = NULL; - char* output = NULL; PyObject* command = NULL; PyObject* environ = NULL; PyObject* bind = NULL; - PyObject* logging_callback = NULL; + PyObject* callback = NULL; int nice = 0; - int return_output = 0; - if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|OOOip", kwlist, &command, &environ, - &bind, &logging_callback, &nice, &return_output)) + if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|OOOi", kwlist, &command, &environ, + &bind, &callback, &nice)) return NULL; // Check if command is a list @@ -881,6 +878,12 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* goto ERROR; } + // Check callback + if (callback && !PyCallable_Check(callback)) { + PyErr_SetString(PyExc_TypeError, "callback must be callable\n"); + goto ERROR; + } + // Create jail r = pakfire_jail_create(&jail, self->pakfire, flags); if (r) { @@ -888,14 +891,10 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* goto ERROR; } - // Set logging callback - if (logging_callback) { - if (!PyCallable_Check(logging_callback)) { - PyErr_SetString(PyExc_TypeError, "logging_callback must be callable\n"); - goto ERROR; - } - - pakfire_jail_set_log_callback(jail, __Pakfire_logging_callback, logging_callback); + // Check callback + if (callback && !PyCallable_Check(callback)) { + PyErr_SetString(PyExc_TypeError, "callback must be callable\n"); + goto ERROR; } // Set nice @@ -964,7 +963,8 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* } // Execute command - r = pakfire_jail_exec(jail, argv, (return_output) ? &output : NULL); + r = pakfire_jail_exec_communicate(jail, argv, + NULL, Pakfire_execute_output_callback, callback); // If the return code was negative, we had some internal error if (r < 0) { @@ -984,34 +984,15 @@ static PyObject* Pakfire_execute(PakfireObject* self, PyObject* args, PyObject* // The process has exited successfully - // Did the user request the output? - if (return_output) { - // Return the buffer as bytes - if (output) - ret = PyBytes_FromString(output); - } - - // Otherwise just return None - if (!ret) { - ret = Py_None; - Py_INCREF(ret); - } + // Return None + ret = Py_None; + Py_INCREF(ret); ERROR: if (argv) free(argv); - if (output) - free(output); - - if (jail) { - // Dereference the logging callback - // It might happen that the jail is not being freed because something else is - // holding a reference to it. We will however lose the reference to the logging - // function here which is why we reset it. - pakfire_jail_set_log_callback(jail, NULL, NULL); - + if (jail) pakfire_jail_unref(jail); - } return ret; } diff --git a/src/libpakfire/include/pakfire/jail.h b/src/libpakfire/include/pakfire/jail.h index d96782118..b6b1eb901 100644 --- a/src/libpakfire/include/pakfire/jail.h +++ b/src/libpakfire/include/pakfire/jail.h @@ -29,18 +29,11 @@ enum pakfire_jail_flags { PAKFIRE_JAIL_NONE = 0, }; -typedef int (*pakfire_jail_log_callback)(struct pakfire* pakfire, void* data, - int priority, const char* line, size_t length); - int pakfire_jail_create(struct pakfire_jail** jail, struct pakfire* pakfire, int flags); struct pakfire_jail* pakfire_jail_ref(struct pakfire_jail* jail); struct pakfire_jail* pakfire_jail_unref(struct pakfire_jail* jail); -// Logging -int pakfire_jail_set_log_callback(struct pakfire_jail* jail, - pakfire_jail_log_callback callback, void* data); - // Mountpoints int pakfire_jail_bind(struct pakfire_jail* jail, const char* source, const char* target, int flags); @@ -58,6 +51,19 @@ int pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[], char** outp int pakfire_jail_exec_script(struct pakfire_jail* jail, const char* script, const size_t size, const char* args[], char** output); +// Communicate +typedef int (*pakfire_jail_communicate_in) + (struct pakfire* pakfire, void* data, int fd); +typedef int (*pakfire_jail_communicate_out) + (struct pakfire* pakfire, void* data, int priority, const char* line, const size_t length); + +int pakfire_jail_exec_communicate( + struct pakfire_jail* jail, + const char* argv[], + pakfire_jail_communicate_in callback_in, + pakfire_jail_communicate_out callback_out, + void* data); + #ifdef PAKFIRE_PRIVATE #include diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c index d3899940a..0894d81e9 100644 --- a/src/libpakfire/jail.c +++ b/src/libpakfire/jail.c @@ -96,10 +96,6 @@ struct pakfire_jail { // Environment char* env[ENVIRON_SIZE]; - // Logging - pakfire_jail_log_callback log_callback; - void* log_data; - // Mountpoints struct pakfire_jail_mountpoint mountpoints[MAX_MOUNTPOINTS]; unsigned int num_mountpoints; @@ -123,6 +119,7 @@ struct pakfire_jail_exec { // Log pipes struct pakfire_jail_pipes { + int stdin[2]; int stdout[2]; int stderr[2]; @@ -132,6 +129,13 @@ struct pakfire_jail_exec { int log_DEBUG[2]; } pipes; + // Communicate + struct pakfire_jail_communicate { + pakfire_jail_communicate_in in; + pakfire_jail_communicate_out out; + void* data; + } communicate; + // Log buffers struct pakfire_jail_buffers { struct pakfire_log_buffer stdout; @@ -237,11 +241,6 @@ PAKFIRE_EXPORT int pakfire_jail_create(struct pakfire_jail** jail, DEBUG(j->pakfire, "Allocated new jail at %p\n", j); - // Set default log callback - r = pakfire_jail_set_log_callback(j, pakfire_jail_default_log_callback, NULL); - if (r) - goto ERROR; - // Set default environment for (const struct environ* e = ENV; e->key; e++) { r = pakfire_jail_set_env(j, e->key, e->val); @@ -412,16 +411,6 @@ PAKFIRE_EXPORT int pakfire_jail_import_env(struct pakfire_jail* jail, const char return 0; } -// Logging - -PAKFIRE_EXPORT int pakfire_jail_set_log_callback(struct pakfire_jail* jail, - pakfire_jail_log_callback callback, void* data) { - jail->log_callback = callback; - jail->log_data = data; - - return 0; -} - /* This function replaces any logging in the child process. @@ -468,7 +457,7 @@ static int pakfire_jail_log_buffer_is_full(const struct pakfire_log_buffer* buff */ static int pakfire_jail_handle_log(struct pakfire_jail* jail, struct pakfire_jail_exec* ctx, int priority, int fd, - struct pakfire_log_buffer* buffer, pakfire_jail_log_callback callback, void* data) { + struct pakfire_log_buffer* buffer, pakfire_jail_communicate_out callback, void* data) { char line[BUFFER_SIZE + 1]; // Fill up buffer from fd @@ -635,7 +624,7 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec int fd = events[i].data.fd; struct pakfire_log_buffer* buffer = NULL; - pakfire_jail_log_callback callback = NULL; + pakfire_jail_communicate_out callback = NULL; void* data = NULL; int priority; @@ -679,15 +668,15 @@ static int pakfire_jail_wait(struct pakfire_jail* jail, struct pakfire_jail_exec buffer = &ctx->buffers.stdout; priority = LOG_INFO; - callback = jail->log_callback; - data = jail->log_data; + callback = ctx->communicate.out; + data = ctx->communicate.data; } else if (fd == stderr) { buffer = &ctx->buffers.stderr; priority = LOG_ERR; - callback = jail->log_callback; - data = jail->log_data; + callback = ctx->communicate.out; + data = ctx->communicate.data; } else { DEBUG(jail->pakfire, "Received invalid file descriptor %d\n", fd); @@ -724,8 +713,8 @@ static int pakfire_jail_capture_stdout(struct pakfire* pakfire, void* data, int char** output = (char**)data; int r; - // Append everything from stdout to a buffer - if (priority == LOG_INFO) { + // Append everything from stdout to a buffer + if (output && priority == LOG_INFO) { r = asprintf(output, "%s%s", (output && *output) ? *output : "", line); if (r < 0) return 1; @@ -1135,6 +1124,31 @@ static int pakfire_jail_wait_for_signal(struct pakfire_jail* jail, int fd) { return r; } +static int pakfire_jail_stream_stdin(struct pakfire_jail* jail, struct pakfire_jail_exec* ctx) { + int r; + + // Nothing to do if there is no stdin callback set + if (!ctx->communicate.in) { + DEBUG(jail->pakfire, "Callback for standard input is not set\n"); + return 0; + } + + int* fd = &ctx->pipes.stdin[1]; + + DEBUG(jail->pakfire, "Streaming standard input...\n"); + + // Calling the callback + r = ctx->communicate.in(jail->pakfire, ctx->communicate.data, *fd); + + DEBUG(jail->pakfire, "Standard input callback finished: %d\n", r); + + // Close the file descriptor when we are done + close(*fd); + *fd = 0; + + return r; +} + /* Performs the initialisation that needs to happen in the parent part */ @@ -1164,6 +1178,11 @@ static int pakfire_jail_parent(struct pakfire_jail* jail, struct pakfire_jail_ex if (r) return r; + // Stream standard input + r = pakfire_jail_stream_stdin(jail, ctx); + if (r) + return r; + return 0; } @@ -1186,10 +1205,6 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe DEBUG(jail->pakfire, "Launched child process in jail with PID %d\n", pid); - // Log argv - for (unsigned int i = 0; argv[i]; i++) - DEBUG(jail->pakfire, " argv[%d] = %s\n", i, argv[i]); - // Wait for the parent to finish initialization r = pakfire_jail_wait_for_signal(jail, ctx->completed_fd); if (r) @@ -1267,6 +1282,17 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe close(ctx->pipes.log_DEBUG[0]); #endif /* ENABLE_DEBUG */ + // Connect standard input + if (ctx->pipes.stdin[0]) { + r = dup2(ctx->pipes.stdin[0], STDIN_FILENO); + if (r < 0) { + ERROR(jail->pakfire, "Could not connect fd %d to stdin: %m\n", + ctx->pipes.stdin[0]); + + return 1; + } + } + // Connect standard output and error if (ctx->pipes.stdout[1] && ctx->pipes.stderr[1]) { r = dup2(ctx->pipes.stdout[1], STDOUT_FILENO); @@ -1286,6 +1312,7 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe } // Close the pipe (as we have moved the original file descriptors) + pakfire_jail_close_pipe(jail, ctx->pipes.stdin); pakfire_jail_close_pipe(jail, ctx->pipes.stdout); pakfire_jail_close_pipe(jail, ctx->pipes.stderr); } @@ -1305,6 +1332,13 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe if (r) return r; + DEBUG(jail->pakfire, "Child process initialization done\n"); + DEBUG(jail->pakfire, "Launching command:\n"); + + // Log argv + for (unsigned int i = 0; argv[i]; i++) + DEBUG(jail->pakfire, " argv[%d] = %s\n", i, argv[i]); + // exec() command r = execvpe(argv[0], (char**)argv, jail->env); if (r < 0) @@ -1326,7 +1360,10 @@ static int pakfire_jail_child(struct pakfire_jail* jail, struct pakfire_jail_exe // Run a command in the jail static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[], - const int interactive) { + const int interactive, + pakfire_jail_communicate_in communicate_in, + pakfire_jail_communicate_out communicate_out, + void* data) { int exit = -1; int r; @@ -1336,11 +1373,22 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[], return -1; } + // Send any output to the default logger if no callback is set + if (!communicate_out) + communicate_out = pakfire_jail_default_log_callback; + // Initialize context for this call struct pakfire_jail_exec ctx = { .pipes = { - .stdout = { 0, 0 }, - .stderr = { 0, 0 }, + .stdin = { 0, 0 }, + .stdout = { 0, 0 }, + .stderr = { 0, 0 }, + }, + + .communicate = { + .in = communicate_in, + .out = communicate_out, + .data = data, }, }; @@ -1358,6 +1406,13 @@ 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 (!interactive) { + // stdin (only if callback is set) + if (ctx.communicate.in) { + r = pakfire_jail_setup_pipe(jail, &ctx.pipes.stdin, 0); + if (r) + goto ERROR; + } + // stdout r = pakfire_jail_setup_pipe(jail, &ctx.pipes.stdout, 0); if (r) @@ -1480,6 +1535,7 @@ ERROR: } // Close any file descriptors + pakfire_jail_close_pipe(jail, ctx.pipes.stdin); pakfire_jail_close_pipe(jail, ctx.pipes.stdout); pakfire_jail_close_pipe(jail, ctx.pipes.stderr); if (ctx.pidfd) @@ -1491,23 +1547,22 @@ ERROR: return exit; } +PAKFIRE_EXPORT int pakfire_jail_exec_communicate( + struct pakfire_jail* jail, + const char* argv[], + pakfire_jail_communicate_in callback_in, + pakfire_jail_communicate_out callback_out, + void* data) { + return __pakfire_jail_exec(jail, argv, 0, callback_in, callback_out, data); +} + PAKFIRE_EXPORT int pakfire_jail_exec(struct pakfire_jail* jail, const char* argv[], char** output) { int r; - // Store logging callback - pakfire_jail_log_callback log_callback = jail->log_callback; - void* log_data = jail->log_data; - - // Capture output if requested by user - if (output) - pakfire_jail_set_log_callback(jail, pakfire_jail_capture_stdout, output); - // Run exec() - r = __pakfire_jail_exec(jail, argv, 0); - - // Restore log callback - pakfire_jail_set_log_callback(jail, log_callback, log_data); + r = __pakfire_jail_exec(jail, argv, 0, + NULL, pakfire_jail_capture_stdout, output); return r; } @@ -1521,7 +1576,7 @@ static int pakfire_jail_exec_interactive( if (r) return r; - return __pakfire_jail_exec(jail, argv, 1); + return __pakfire_jail_exec(jail, argv, 1, NULL, NULL, NULL); } PAKFIRE_EXPORT int pakfire_jail_exec_script(struct pakfire_jail* jail, diff --git a/src/libpakfire/libpakfire.sym b/src/libpakfire/libpakfire.sym index 5d7eaf45b..0b9795428 100644 --- a/src/libpakfire/libpakfire.sym +++ b/src/libpakfire/libpakfire.sym @@ -151,13 +151,13 @@ global: pakfire_jail_bind; pakfire_jail_create; pakfire_jail_exec; + pakfire_jail_exec_communicate; pakfire_jail_exec_script; pakfire_jail_get_env; pakfire_jail_import_env; pakfire_jail_nice; pakfire_jail_ref; pakfire_jail_set_env; - pakfire_jail_set_log_callback; pakfire_jail_unref; # log diff --git a/tests/libpakfire/jail.c b/tests/libpakfire/jail.c index 59431d50d..0bb0348c1 100644 --- a/tests/libpakfire/jail.c +++ b/tests/libpakfire/jail.c @@ -294,6 +294,46 @@ FAIL: return r; } +static int callback_stdin(struct pakfire* pakfire, void* data, int fd) { + const char* lines[] = { "a", "b", "c", NULL }; + int r; + + for (const char** line = lines; *line; line++) { + r = dprintf(fd, "%s\n", *line); + if (r < 0) { + LOG_ERROR("Could not write line (%s) to stdin: %m\n", *line); + + return 1; + } + } + + return 0; +} + +static int test_communicate(const struct test* t) { + struct pakfire_jail* jail = NULL; + int r = EXIT_FAILURE; + + const char* argv[] = { + "/command", "pipe", NULL, + }; + + // Create a new jail + ASSERT_SUCCESS(pakfire_jail_create(&jail, t->pakfire, 0)); + + // Check if the mount actually works + ASSERT_SUCCESS(pakfire_jail_exec_communicate(jail, argv, callback_stdin, NULL, NULL)); + + // Success + r = EXIT_SUCCESS; + +FAIL: + if (jail) + pakfire_jail_unref(jail); + + return r; +} + int main(int argc, const char* argv[]) { testsuite_add_test(test_create); testsuite_add_test(test_env); @@ -304,6 +344,7 @@ int main(int argc, const char* argv[]) { testsuite_add_test(test_pid_limit); testsuite_add_test(test_file_ownership); testsuite_add_test(test_bind); + testsuite_add_test(test_communicate); return testsuite_run(argc, argv); } -- 2.39.5