From 437fd47c8b95200ce1561b356dece0a8ff532397 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Sep 2026 21:02:35 +0100 Subject: [PATCH 1/5] Add a diagram of the processes we will generally have Work on several issues and PRs like #369, #589, #573 has been impeded by the structure of the process hierarchy not being obvious. We can make this better by assigning clear names to the various long-running processes that might exist. We say that the top-level process (the `bwrap` process that was run by the user), which continues to run outside the sandbox boundary, is the *monitor*. It is responsible for monitoring sandboxed processes and passing on their exit status. Typically the child of the monitor will be an *init* process inside the sandbox (in some cases this is not present). Finally, the sandboxed *command* is the `COMMAND` specified after the `--` divider on bwrap's command-line. Signed-off-by: Simon McVittie --- bubblewrap.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/bubblewrap.c b/bubblewrap.c index 2ab88d2f..598b242f 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -41,6 +41,57 @@ #include "network.h" #include "bind-mount.h" +/* + * PROCESS HIERARCHY + * + * In general the process hierarchy will look like this: + * + * MONITOR: + * original bwrap process outside sandbox blocks in monitor_child() + * | + * - - | (clone) - - - sandbox boundary - - - - - - - - - - - - + * | + * \-- INIT: child in sandbox blocks in do_init() + * | (process 1 in new process ID namespace, if unshared) + * | + * | (fork) + * | + * \-- COMMAND: execve final "payload" process + * + * Exceptions to this: + * + * - If user passed --unshare-pid --as-pid-1, there is no separate + * init process. The sandboxed command is a direct child of the monitor, + * immediately below the sandbox boundary: + * + * MONITOR: blocks in monitor_child() + * | + * - - | (clone) - - - sandbox boundary - - - - - - - - - - - - + * | + * \-- COMMAND: execve final "payload" process as pid 1 + * + * - If user did not --unshare-pid and did not --lock-file or --sync-fd, + * we don't need an init process. + * Again the sandboxed command is a direct child of the monitor. + * + * - If user did not --unshare-pid but did use --lock-file or --sync-fd, + * then the init process is not actually process 1, + * but still reaps child processes and holds the locks/sync-fd open + * + * - If user specified --pidns, the child process of the clone() call + * (the "intermediate child") will fork and then exit. + * The process that becomes sandboxed is initially a grandchild of + * the monitor, which is re-parented to the monitor, because in + * this case the monitor is a subreaper. + * After this re-parenting, the sandboxed process becomes either + * init or the command, as above. + * + * - If user specified --pidns *and* --unshare-pid, + * a second layer of intermediate child also forks and exits, + * and the process that becomes sandboxed is initially a great-grandchild + * of the monitor, which (again) is re-parented to the monitor. + */ + #ifndef CLONE_NEWCGROUP #define CLONE_NEWCGROUP 0x02000000 /* New cgroup namespace */ #endif From 95e34df03e762afbad4e4f3174f8da876312f978 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Sep 2026 21:03:22 +0100 Subject: [PATCH 2/5] Add more debug logging to clarify which process is which This uses the terminology from the diagram added in the previous commit. Signed-off-by: Simon McVittie --- bubblewrap.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bubblewrap.c b/bubblewrap.c index 598b242f..f7ece2c1 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -547,6 +547,8 @@ monitor_child (int event_fd, pid_t child_pid, int setup_finished_fd) pid_t died_pid; int died_status; + debug ("monitor[%d] outside sandbox: watching child %d", getpid (), child_pid); + /* Close all extra fds in the monitoring process. Any passed in fds have been passed on to the child anyway. */ if (event_fd != -1) @@ -638,6 +640,8 @@ do_init (int event_fd, pid_t initial_pid) int initial_exit_status = 1; LockFile *lock; + debug ("init[%d] in sandbox: watching command %d", getpid (), initial_pid); + for (lock = lock_files; lock != NULL; lock = lock->next) { int fd = TEMP_FAILURE_RETRY (open (lock->path, O_RDONLY | O_NOCTTY | O_CLOEXEC)); @@ -3243,6 +3247,8 @@ main (int argc, if (opt_pidns_fd != -1) { + debug ("switching to pid namespace from --pidns"); + if (setns (opt_pidns_fd, CLONE_NEWPID) != 0) die_with_error ("Setting pidns failed"); @@ -3252,6 +3258,8 @@ main (int argc, /* We might both have specified an --pidns *and* --unshare-pid, so set up a new child pid namespace under the specified one */ if (opt_unshare_pid) { + debug ("creating new pid namespace below --pidns"); + if (unshare (CLONE_NEWPID)) die_with_error ("unshare pid ns"); @@ -3261,6 +3269,7 @@ main (int argc, /* We're back, either in a child or grandchild, so message the actual pid to the monitor */ + debug ("communicating sandboxed process's new pid to monitor"); close (intermediate_pids_sockets[0]); send_pid_on_socket (intermediate_pids_sockets[1]); close (intermediate_pids_sockets[1]); @@ -3287,6 +3296,7 @@ main (int argc, close (opt_json_status_fd); /* Wait for the parent to init uid/gid maps and drop caps */ + debug ("waiting for monitor to be ready"); res = read (child_wait_fd, &val, 8); close (child_wait_fd); @@ -3294,6 +3304,7 @@ main (int argc, * required permitted caps. This allow us to do full setup as * the user uid, which makes e.g. fuse access work. */ + debug ("setting up sandbox"); switch_to_user_with_privs (); if (opt_unshare_net) @@ -3478,6 +3489,7 @@ main (int argc, } /* All privileged ops are done now, so drop caps we don't need */ + debug ("sandbox setup finished, dropping privileges"); drop_privs (true); if (opt_block_fd != -1) @@ -3564,7 +3576,7 @@ main (int argc, } } - debug ("launch executable %s", argv[0]); + debug ("command[%d] in sandbox: launch executable %s", getpid (), argv[0]); if (proc_fd != -1) close (proc_fd); From d5bf690c1c9105e052e6d1634fa502cccd680fd6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Sep 2026 21:05:15 +0100 Subject: [PATCH 3/5] Fix a misleading debug message It's misleading to say we're `forking for child` if we will not, in fact, fork. Instead, say that we're `forking init parent for command` if that's the case, or say that `init not needed` if we won't fork. This uses the terminology from the diagram I added previously. Signed-off-by: Simon McVittie --- bubblewrap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bubblewrap.c b/bubblewrap.c index f7ece2c1..d00965f6 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -3538,8 +3538,6 @@ main (int argc, if (label_exec (opt_exec_label) == -1) die_with_error ("label_exec %s", argv[0]); - debug ("forking for child"); - if (!opt_as_pid_1 && (opt_unshare_pid || lock_files != NULL || opt_sync_fd != -1)) { /* We have to have a pid 1 in the pid namespace, because @@ -3548,6 +3546,8 @@ main (int argc, * need some process to own these. */ + debug ("forking init parent for command"); + pid = fork (); if (pid == -1) die_with_error ("Can't fork for pid 1"); @@ -3575,6 +3575,10 @@ main (int argc, return do_init (event_fd, pid); } } + else + { + debug ("init not needed, command will be a child of the monitor"); + } debug ("command[%d] in sandbox: launch executable %s", getpid (), argv[0]); From 87cedd18223037736452f2e0d03d850e6eb18d44 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Sep 2026 21:04:05 +0100 Subject: [PATCH 4/5] Make debug() expand to something non-empty, even if disabled This is the standard trick to make a macro always have non-empty contents for syntax purposes, even if it ends up compiling to no machine code. Signed-off-by: Simon McVittie --- utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils.h b/utils.h index 0872223a..6d4dc27e 100644 --- a/utils.h +++ b/utils.h @@ -37,7 +37,7 @@ extern bool bwrap_is_debugging; #define debug(...) bwrap_log (LOG_DEBUG, __VA_ARGS__) #else -#define debug(...) +#define debug(...) do {} while (0) #endif #define UNUSED __attribute__((__unused__)) From e7734569ccece3d3a163ae32f36c576a108d411e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Sep 2026 21:05:31 +0100 Subject: [PATCH 5/5] Try to set the process name for bwrap's internal processes This uses the terminology from the diagram I added previously. Signed-off-by: Simon McVittie --- bubblewrap.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bubblewrap.c b/bubblewrap.c index d00965f6..dd1e6065 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -549,6 +549,10 @@ monitor_child (int event_fd, pid_t child_pid, int setup_finished_fd) debug ("monitor[%d] outside sandbox: watching child %d", getpid (), child_pid); + /* This is just nice-to-have, so don't crash out if this fails */ + if (prctl (PR_SET_NAME, "bwrap monitor") < 0) + debug ("Unable to set process name: %s", strerror (errno)); + /* Close all extra fds in the monitoring process. Any passed in fds have been passed on to the child anyway. */ if (event_fd != -1) @@ -642,6 +646,10 @@ do_init (int event_fd, pid_t initial_pid) debug ("init[%d] in sandbox: watching command %d", getpid (), initial_pid); + /* This is just nice-to-have, so don't crash out if this fails */ + if (prctl (PR_SET_NAME, "bwrap init") < 0) + debug ("Unable to set process name: %s", strerror (errno)); + for (lock = lock_files; lock != NULL; lock = lock->next) { int fd = TEMP_FAILURE_RETRY (open (lock->path, O_RDONLY | O_NOCTTY | O_CLOEXEC));