search for: nbd_internal_fork_safe_perror

V3 was here: <http://mid.mail-archive.com/20230215141158.2426855-1-lersek at redhat.com>. See the Notes section on each patch for the v4 updates. The series is nearly ready for merging: every patch has at least one R-b tag, except "socket activation: avoid manipulating the sign bit". The series builds, and passes "make check" and "make check-valgrind", at

...+ goto uninit_execvpe; pid = fork (); if (pid == -1) { set_error (errno, "fork"); goto empty_env; } if (pid == 0) { /* child - run command */ if (s != FIRST_SOCKET_ACTIVATION_FD) { if (dup2 (s, FIRST_SOCKET_ACTIVATION_FD) == -1) { nbd_internal_fork_safe_perror ("dup2"); @@ -189,45 +196,47 @@ CONNECT_SA.START: 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. */ if (signal (SIGPIPE, SIG_D...

...oto close_socket_pair; + parentfd_transferred = true; + pid = fork (); if (pid == -1) { set_error (errno, "fork"); - goto close_socket_pair; + goto close_high_level_socket; } if (pid == 0) { /* child - run command */ 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_INTERNA...

...ator/states-connect-socket-activation.c index b5e146539cc8..729f37d897fb 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -181,11 +181,11 @@ CONNECT_SA.START: 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, flags & ~FD_CLOEXEC) == -1) { + if (fcntl (s, F_SETFD, (int)(flags & ~(unsigned)FD_CLOEXEC)) == -1) { nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); _exit (126);...

...c b/generator/states-connect.c index 7a828f4..04e894c 100644 --- a/generator/states-connect.c +++ b/generator/states-connect.c @@ -259,8 +259,11 @@ STATE_MACHINE { signal (SIGPIPE, SIG_DFL); execvp (h->argv[0], h->argv); - perror (h->argv[0]); - _exit (EXIT_FAILURE); + nbd_internal_fork_safe_perror (h->argv[0]); + if (errno == ENOENT) + _exit (127); + else + _exit (126); } /* Parent. */ diff --git a/lib/internal.h b/lib/internal.h index bdb0e83..31bc3d4 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -384,5 +384,7 @@ extern void nbd_internal_hexdump (const void...

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

This is version 4 of the following sub-series: [libnbd PATCH v3 06/29] lib/utils: introduce xwrite() as a more robust write() [libnbd PATCH v3 07/29] lib/utils: add async-signal-safe assert() [libnbd PATCH v3 08/29] lib/utils: add unit test for async-signal-safe assert() http://mid.mail-archive.com/20230215141158.2426855-7-lersek at redhat.com

...> + } > + else { > + /* We must unset CLOEXEC on the fd. (dup2 above does this > + * implicitly because CLOEXEC is set on the fd, not on the > + * socket). > + */ > + flags = fcntl (s, F_GETFD, 0); > + if (flags == -1) { > + nbd_internal_fork_safe_perror ("fcntl: F_GETFD"); > + _exit (126); > + } > + if (fcntl (s, F_SETFD, flags & ~FD_CLOEXEC) == -1) { > + nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); > + _exit (126); > + } > + } > + Looks correct. > +...

...f HAVE_EXECVPE int s; struct sockaddr_un addr; char **env; @@ -200,7 +200,8 @@ STATE_MACHINE { /* Restore SIGPIPE back to SIG_DFL. */ signal (SIGPIPE, SIG_DFL); - execvpe (h->argv[0], h->argv, env); + environ = env; + execvp (h->argv[0], h->argv); nbd_internal_fork_safe_perror (h->argv[0]); if (errno == ENOENT) _exit (127); @@ -217,11 +218,4 @@ STATE_MACHINE { memcpy (&h->connaddr, &addr, h->connaddrlen); SET_NEXT_STATE (%^CONNECT.START); return 0; - -#else /* !HAVE_EXECVPE */ - SET_NEXT_STATE (%.DEAD); - set_error (ENOTSUP, &...

...IRST_SOCKET_ACTIVATION_FD); + close (s); + } + else { + /* We must unset CLOEXEC on the fd. (dup2 above does this + * implicitly because CLOEXEC is set on the fd, not on the + * socket). + */ + flags = fcntl (s, F_GETFD, 0); + if (flags == -1) { + nbd_internal_fork_safe_perror ("fcntl: F_GETFD"); + _exit (126); + } + if (fcntl (s, F_SETFD, flags & ~FD_CLOEXEC) == -1) { + nbd_internal_fork_safe_perror ("fcntl: F_SETFD"); + _exit (126); + } + } + + char buf[32]; + const char *v = + nbd_internal_fork_...

...t to assert that bufsize is large enough? (Or rather, abort() if it is not, since assert() is borderline in fork-safe context) > +} > + > +/* Fork-safe version of perror. ONLY use this after fork and before > + * exec, the rest of the time use set_error(). > + */ > +void > +nbd_internal_fork_safe_perror (const char *s) > +{ > + const int err = errno; > + > +#if defined(__GNUC__) > +#pragma GCC diagnostic push > +#pragma GCC diagnostic ignored "-Wunused-result" > +#endif > + write (2, s, strlen (s)); Surprisingly, strlen() is not listed in current POSIX' lis...

This adds coalescing of adjacent extents of the same type, as mentioned by Eric Blake in the commit message here: https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd The patch series is rather long because it detours through adding the <vector.h> library from nbdkit into libnbd and replacing ad hoc uses of realloc, char ** etc in various places. Rich.

V4 was here (incorrectly versioned on the mailing list as v3): <http://mid.mail-archive.com/20230323121016.1442655-1-lersek at redhat.com>. See the Notes section on each patch for the v5 updates. Laszlo Ersek (2): socket activation: generalize environment construction socket activation: set LISTEN_FDNAMES Richard W.M. Jones (2): common/include: Copy ascii-ctype functions from nbdkit