Laszlo Ersek
2023-Feb-15 14:11 UTC
[Libguestfs] [libnbd PATCH v3 22/29] CONNECT_COMMAND.START: check syscalls for errors in the child process
It's bad hygiene to just assume dup2(), close() and signal() will succeed.
Check all return values.
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
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. */
Richard W.M. Jones
2023-Feb-15 17:04 UTC
[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 at redhat.com> > --- > > 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 at redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html