pv-wrap: Don't register stdout, stderr, --pass-fd with FlatpakBwrap
-
pv-wrap: Don't register stdout, stderr, --pass-fd with FlatpakBwrap
This fixes two separate issues.
If we add a file descriptor to the FlatpakBwrap object with flatpak_bwrap_add_fd, then flatpak_bwrap_child_setup() will lseek() back to byte 0 before it execs the replacement process. This is often what we want - for example, when we have written out a file containing the container identification, we want any subsequent read from that file to start at byte 0.
However, it is absolutely not what we want if stdout/stderr are directed to a log file, in which case any writes after the lseek() will start from the beginning and overwrite the content of the log file. The symptom for this is that debug messages from pv-adverb start at the beginning of the log file, overwriting earlier debug messages from pv-wrap (but directing the log to a pipe or to the systemd Journal works fine, because those aren't seekable). Similarly, if --pass-fd is being used for a seekable file, we probably don't want to force it to seek back to byte 0.
The second issue is that adding a fd with flatpak_bwrap_add_fd() means the FlatpakBwrap object takes responsibility for closing that fd. This was incorrect to do for original_stdout and original_stderr without transferring ownership (which we don't want to do), because they are declared with glnx_autofd, meaning that they will be closed automatically when they go out of scope. In the error code path where pv_bwrap_execve() fails, this means they will be closed twice - once by glnx_autofd, and once by the g_autoptr(FlatpakBwrap) - leading to an assertion failure. This is mitigated by not being a problem in the success code path where pv_bwrap_execve() succeeds, which in practice is the one we always want to reach.
Avoid both of these by using a separate list of fds to inherit, not seeking back to the beginning of those, and not closing them.
This has the side-effect that fds from --pass-fd are also not closed, but that seems harmless, and arguably more correct: they're open on entry to main(), and they're still open on exit from main(), so no change of ownership has happened.
steamrt/tasks#370
Fixes: 779d13fd "pv-wrap: Let pv-adverb restore our original stdout/stderr"