Open smcv opened 3 weeks ago
Another unrelated thought: systemd uses a
safe_close()
wrapper instead of directlyclose(3)
to assertEBADF
was not returned, to spot file descriptor mixups,
GLib does similarly, in g_close()
. A potential problem with that technique is that the assertion is not going to be async-signal-safe, which is troublesome in a process that forks. On the other hand, bubblewrap is single-threaded, so it's usually OK for us to call non-async-signal-safe functions after fork()
.
Use PIPE_READ_END, PIPE_WRITE_END to clarify use of a socketpair
Both sockets in the socket pair are technically bidirectional, but we are using them as though they were unidirectional, with the same convention as pipe(): the first socket in the array is used for reading, and the second is for writing.
Close unused ends of intermediate_pids_sockets sooner
Instead of making this conditional and keeping track of the correct condition under which to call it, we can use cleanup_fdp(), which is a no-op when called with a pointer to a negative number, to close the socket unconditionally.
In the parent bwrap monitor process (outside the sandbox), we never want to use the write end (which is reserved for the child), so we can and should close it as soon as we have forked.
Conversely, in the child process, we never want to use the read end (which is reserved for the parent), so we should close that as soon as we know we are in the child.
Use cleanup_fd to close intermediate pid sockets
This closes the fd and sets the variable to -1 as a single operation, which is easier to reason about because it does not leave any variables containing dangling references to invalid fds.
cc @refi64 @WGH- @mcatanzaro
I think #576 would have better clarity if it was rebased on this and used similar techniques. Reviews welcome, even from non-maintainers.