[Libguestfs] [libnbd PATCH v3 15/19] CONNECT_COMMAND.START: replace execvp() call with fork-safe variant

[Laszlo Ersek]

Per POSIX, execvp() is not safe to call in a child process forked from a
multi-threaded process. We can now replace the execvp() call in the child
process with a call to our fork-safe (async-signal-safe) variant.

Prepare our internal execvpe context on the parent's construction path,
use the context in the child, and release the context in the parent on the
way out, regardless of whether the handler as a whole succeeded or not.
(The context is only a temporary resource.)

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---

Notes:
    v4:
    
    - pick up Rich's R-b
    
    context:-U7

 generator/states-connect.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/generator/states-connect.c b/generator/states-connect.c
index 4dce7e01f457..65a68003583e 100644
--- a/generator/states-connect.c
+++ b/generator/states-connect.c
@@ -32,14 +32,16 @@
 #include <signal.h>
 #include <netdb.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 
+extern char **environ;
+
 /* Disable Nagle's algorithm on the socket, but don't fail. */
 static void
 disable_nagle (int sock)
 {
   const int flag = 1;
 
   setsockopt (sock, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof flag);
@@ -239,14 +241,15 @@  CONNECT_TCP.NEXT_ADDRESS:
 
  CONNECT_COMMAND.START:
   enum state next;
   bool parentfd_transferred;
   int sv[2];
   int flags;
   struct socket *sock;
+  struct execvpe execvpe_ctx;
   pid_t pid;
 
   assert (!h->sock);
   assert (h->argv.ptr);
   assert (h->argv.ptr[0]);
 
   next = %.DEAD;
@@ -278,18 +281,24 @@  CONNECT_COMMAND.START:
 
   sock = nbd_internal_socket_create (sv[0]);
   if (!sock)
     /* nbd_internal_socket_create() calls set_error() internally */
     goto close_socket_pair;
   parentfd_transferred = true;
 
+  if (nbd_internal_execvpe_init (&execvpe_ctx, h->argv.ptr[0], h->argv.len) ==
+      -1) {
+    set_error (errno, "nbd_internal_execvpe_init");
+    goto close_high_level_socket;
+  }
+
   pid = fork ();
   if (pid == -1) {
     set_error (errno, "fork");
-    goto close_high_level_socket;
+    goto uninit_execvpe;
   }
 
   if (pid == 0) {         /* child - run command */
     if (close (sv[0]) == -1) {
       nbd_internal_fork_safe_perror ("close");
       _exit (126);
     }
@@ -307,15 +316,15 @@  CONNECT_COMMAND.START:
 
     /* Restore SIGPIPE back to 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);
+    (void)nbd_internal_fork_safe_execvpe (&execvpe_ctx, &h->argv, environ);
     nbd_internal_fork_safe_perror (h->argv.ptr[0]);
     if (errno == ENOENT)
       _exit (127);
     else
       _exit (126);
   }
 
@@ -326,14 +335,17 @@  CONNECT_COMMAND.START:
   /* The sockets are connected already, we can jump directly to
    * receiving the server magic.
    */
   next = %^MAGIC.START;
 
   /* fall through, for releasing the temporaries */
 
+uninit_execvpe:
+  nbd_internal_execvpe_uninit (&execvpe_ctx);
+
 close_high_level_socket:
   if (next == %.DEAD)
     sock->ops->close (sock);
 
 close_socket_pair:
   assert (next == %.DEAD || parentfd_transferred);
   if (!parentfd_transferred)