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

[Paul Holzinger <pholzing@redhat.com>]

On 11/12/2025 04:54, David Gibson 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>
Reviewed-by: Paul Holzinger <pholzing@redhat.com>
> ---
>   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

-- 
Paul Holzinger