Re: [Libguestfs] [libnbd PATCH v3 22/29] CONNECT_COMMAND.START: check syscalls for errors in the child process

On Wed, Feb 15, 2023 at 03:11:51PM +0100, Laszlo Ersek wrote: > It's bad hygiene to just assume dup2(), close() and signal() will succeed. > Check all return values. > > Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; > --- > > Notes: > context:-U10 > > generator/states-connect.c | 22 +++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/generator/states-connect.c b/generator/states-connect.c > index e08608336fd6..d2476dc11c60 100644 > --- a/generator/states-connect.c > +++ b/generator/states-connect.c > @@ -262,29 +262,41 @@ CONNECT_COMMAND.START: > > pid = fork (); > if (pid == -1) { > SET_NEXT_STATE (%.DEAD); > set_error (errno, "fork"); > close (sv[0]); > close (sv[1]); > return 0; > } > if (pid == 0) { /* child - run command */ > - close (sv[0]); > - dup2 (sv[1], STDIN_FILENO); > - dup2 (sv[1], STDOUT_FILENO); > + if (close (sv[0]) == -1) { > + nbd_internal_fork_safe_perror ("close"); > + _exit (126); > + } > + if (dup2 (sv[1], STDIN_FILENO) == -1 || > + dup2 (sv[1], STDOUT_FILENO) == -1) { > + nbd_internal_fork_safe_perror ("dup2"); > + _exit (126); > + } > NBD_INTERNAL_FORK_SAFE_ASSERT (sv[1] != STDIN_FILENO); > NBD_INTERNAL_FORK_SAFE_ASSERT (sv[1] != STDOUT_FILENO); > - close (sv[1]); > + if (close (sv[1]) == -1) { > + nbd_internal_fork_safe_perror ("close"); > + _exit (126); > + } > > /* Restore SIGPIPE back to SIG_DFL. */ > - signal (SIGPIPE, SIG_DFL); > + if (signal (SIGPIPE, SIG_DFL) == SIG_ERR) { > + nbd_internal_fork_safe_perror ("signal"); > + _exit (126); > + } > > execvp (h->argv.ptr[0], h->argv.ptr); > nbd_internal_fork_safe_perror (h->argv.ptr[0]); > if (errno == ENOENT) > _exit (127); > else > _exit (126); > } > > /* Parent. */ For use of exit code 126, see: https://listman.redhat.com/archives/libguestfs/2019-September/022737.html https://listman.redhat.com/archives/libguestfs/2019-September/022739.html https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html

Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com&gt;