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 <michael.tremer@ipfire.org>

diff --git a/src/_pakfire/pakfire.c b/src/_pakfire/pakfire.c
index 3843d94068c0d2a90b4c920e23bffe592b519948..ea25f3ed2fad8ca31c9ee21a83244d2d0a290eec 100644 (file)
--- 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 d96782118adf38ee3a55ee9ae0478e3bfa1e3e21..b6b1eb90100aa93adedb29619e45a4541e8fef26 100644 (file)
--- 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 <pakfire/cgroup.h>

diff --git a/src/libpakfire/jail.c b/src/libpakfire/jail.c
index d3899940a4b514290040e79f68fc7d60bd53b2a1..0894d81e9f11a5550f7db9ec5bdb713e89ea25cb 100644 (file)
--- 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 5d7eaf45b34ff69169884419625b444afb4430b9..0b97954287fa3ff0232ebd131b71cca428bc26cb 100644 (file)
--- 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 59431d50ddaac00bf7d731d2a4fb332d91124ec6..0bb0348c115ca007870939a29680d0b61fe2cb6f 100644 (file)
--- 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);
 }