Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command-pid member to --json-status-fd #576

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions bubblewrap.c
Original file line number Diff line number Diff line change
Expand Up @@ -2654,6 +2654,7 @@ main (int argc,
int clone_flags;
char *old_cwd = NULL;
pid_t pid;
pid_t command_pid;
int event_fd = -1;
int child_wait_fd = -1;
int setup_finished_pipe[] = {-1, -1};
Expand All @@ -2665,6 +2666,7 @@ main (int argc,
int res UNUSED;
cleanup_free char *args_data UNUSED = NULL;
int intermediate_pids_sockets[2] = {-1, -1};
int command_pid_sockets[2] = {-1, -1};
const char *exec_path = NULL;

/* Handle --version early on before we try to acquire/drop
Expand Down Expand Up @@ -2897,6 +2899,14 @@ main (int argc,
create_pid_socketpair (intermediate_pids_sockets);
}

/* Sometimes we spawn command as immediate child, sometimes we run pid 1, sometimes there are more intermediate pids...
* For simplicity, get pid just before exec'ing command to get the real pid.
**/
if (opt_json_status_fd != -1)
{
create_pid_socketpair (command_pid_sockets);
}

pid = raw_clone (clone_flags, NULL);
if (pid == -1)
{
Expand Down Expand Up @@ -2986,6 +2996,16 @@ main (int argc,
/* Ignore res, if e.g. the child died and closed child_wait_fd we don't want to error out here */
close (child_wait_fd);

close (command_pid_sockets[1]);
command_pid = read_pid_from_socket (command_pid_sockets[0]);
close (command_pid_sockets[0]);

if (opt_json_status_fd != -1)
{
cleanup_free char *output = xasprintf ("{ \"command-pid\": %i }\n", command_pid);
dump_info (opt_json_status_fd, output, TRUE);
}
Comment on lines +2999 to +3007
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make the uppermost process (the monitor process, outside the sandbox) block until the final process sends its pid. And, if the child process (reported as child-pid) fails for any reason, then read_pid_from_socket() will fail with a fatal error, preventing monitor_child() from relaying the true exit status of the child.

I think it would be better to monitor command_pid_sockets[0] in monitor_child(), and use non-blocking I/O for that, so that we output the information when we have it, but we don't wait for it.


return monitor_child (event_fd, pid, setup_finished_pipe[0]);
}

Expand Down Expand Up @@ -3344,6 +3364,13 @@ main (int argc,

__debug__ (("launch executable %s\n", argv[0]));

if (opt_json_status_fd != -1)
{
close (command_pid_sockets[0]);
send_pid_on_socket (command_pid_sockets[1]);
close (command_pid_sockets[1]);
}

if (proc_fd != -1)
close (proc_fd);

Expand Down
8 changes: 8 additions & 0 deletions bwrap.xml
Original file line number Diff line number Diff line change
Expand Up @@ -452,17 +452,25 @@
Multiple JSON documents are written to <arg choice="plain">FD</arg>,
one per line (<ulink url="https://jsonlines.org/">"JSON lines" format</ulink>).
Each line is a single JSON object.
</para><para>
After <command>bwrap</command> has started the child process inside the sandbox,
it writes an object with a <literal>child-pid</literal> member to the
<option>--json-status-fd</option> (this duplicates the older <option>--info-fd</option>).
The corresponding value is the process ID of the child process in the pid namespace from
which <command>bwrap</command> was run.
If available, the namespace IDs are also included in the object with the <literal>child-pid</literal>;
again, this duplicates the older <option>--info-fd</option>.
</para><para>
Just before COMMAND is exec'ed, the process writes its process ID with a <literal>command-pid</literal>
member. This process ID is the PID of COMMAND, and it might not match <literal>child-pid</literal>
mentioned above if <command>bwrap</command> runs additional pid 1 process (e.g. when using
<option>--unshare-pid</option>).
</para><para>
When the child process inside the sandbox exits, <command>bwrap</command> writes an object
with an exit-code member, and then closes the <option>--json-status-fd</option>. The value
corresponding to <literal>exit-code</literal> is the exit status of the child, in the usual
shell encoding (n if it exited normally with status n, or 128+n if it was killed by signal n).
</para><para>
Other members may be added to those objects in future versions of <command>bwrap</command>,
and other JSON objects may be added before or after the current objects, so readers must
ignore members and objects that they do not understand.
Expand Down