Re: [PATCH v2 3/3] pasta: Clean up waiting pasta child on failures

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 11 Dec 2025 14:54:36 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> When pasta is invoked with a command rather than an existing namespace to
> attach to, it spawns a child process to run a shell or other command.  We
> create that process during conf(), since we need the namespace to exist for
> much of our setup.  However, we don't want the specified command to run
> until the pasta network interface is ready for use.  Therefore,
> pasta_spawn_cmd() executing in the child waits before exec()ing.  main()
> signals the child to continue with SIGUSR1 shortly before entering the
> main forwarding loop.
> 
> This has the downside that if we exit due to any kind of failure between
> conf() and the SIGUSR1, the child process will be around waiting
> indefinitely.  The user must manually clean this up.
> 
> Make this cleaner, by having the child use PR_SET_PDEATHSIG to have
> itself killed if the parent dies during this window.  Technically
> speaking this is racy: if the parent dies before the child can call
> the prctl() it will be left zombie-like as before.  However, as long
> as the parent completes pasta_wait_for_ns() before dying, I wasn't
> able to trigger the race.  Since the consequences of this going wrong
> are merely a bit ugly, I think that's good enough.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Is this:

Suggested-by: Paul Holzinger <pholzing@redhat.com>

? In any case, Cc'ing him with full quote to be sure he doesn't miss v2.

> ---
>  pasta.c | 11 +++++++++++
>  util.c  |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/pasta.c b/pasta.c
> index 5c693de1..c307b8a8 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -40,6 +40,7 @@
>  #include <arpa/inet.h>
>  #include <netinet/in.h>
>  #include <net/ethernet.h>
> +#include <sys/prctl.h>
>  #include <sys/syscall.h>
>  #include <linux/magic.h>
>  
> @@ -189,6 +190,10 @@ static int pasta_spawn_cmd(void *arg)
>  	size_t conf_hostname_len;
>  	sigset_t set;
>  
> +	/* If the parent dies with an error, so should we */
> +	if (prctl(PR_SET_PDEATHSIG, SIGKILL))
> +		die_perror("Couldn't set PR_SET_PDEATHSIG");
> +
>  	/* We run in a detached PID and mount namespace: mount /proc over */
>  	if (mount("", "/proc", "proc", 0, NULL))
>  		warn_perror("Couldn't mount /proc");
> @@ -215,6 +220,12 @@ static int pasta_spawn_cmd(void *arg)
>  	sigaddset(&set, SIGUSR1);
>  	sigwaitinfo(&set, NULL);
>  
> +	/* Once exec()ed this process is more valuable, and easier to see and
> +	 * clean up.  Let us outlive our parent now.
> +	 */
> +	if (prctl(PR_SET_PDEATHSIG, 0))
> +		die_perror("Couldn't clear PR_SET_PDEATHSIG");
> +
>  	execvp(a->exe, a->argv);
>  
>  	die_perror("Failed to start command or shell");
> diff --git a/util.c b/util.c
> index da12c962..27303950 100644
> --- a/util.c
> +++ b/util.c
> @@ -35,6 +35,7 @@
>  #include "log.h"
>  #include "pcap.h"
>  #include "epoll_ctl.h"
> +#include "pasta.h"
>  #ifdef HAS_GETRANDOM
>  #include <sys/random.h>
>  #endif

-- 
Stefano