Re: [Libguestfs] [libnbd PATCH v3 14/29] socket activation: check syscalls for errors in the child process

It's bad hygiene to just assume dup2(), close() and signal() will succeed, especially considering the inconsistency that we do check fcntl(). Check all return values. Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: context:-U12 generator/states-connect-socket-activation.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c index 729f37d897fb..766f46177bf3 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -161,50 +161,59 @@ CONNECT_SA.START: } pid = fork (); if (pid == -1) { SET_NEXT_STATE (%.DEAD); set_error (errno, "fork"); close (s); string_vector_empty (&env); return 0; } if (pid == 0) { /* child - run command */ if (s != FIRST_SOCKET_ACTIVATION_FD) { - dup2 (s, FIRST_SOCKET_ACTIVATION_FD); - close (s); + if (dup2 (s, FIRST_SOCKET_ACTIVATION_FD) == -1) { + nbd_internal_fork_safe_perror ("dup2"); + _exit (126); + } + if (close (s) == -1) { + nbd_internal_fork_safe_perror ("close"); + _exit (126); + } } else { /* We must unset CLOEXEC on the fd. (dup2 above does this * implicitly because CLOEXEC is set on the fd, not on the * socket). */ int flags = fcntl (s, F_GETFD, 0); if (flags == -1) { nbd_internal_fork_safe_perror ("fcntl: F_GETFD"); _exit (126); } if (fcntl (s, F_SETFD, (int)(flags & ~(unsigned)FD_CLOEXEC)) == -1) { nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); _exit (126); } } char buf[32]; const char *v = nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); strcpy (&env.ptr[0][PREFIX_LENGTH], v); /* 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); + } environ = env.ptr; 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. */ close (s);