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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3595 bytes --]

On Thu, Dec 11, 2025 at 08:16:45AM +0100, Stefano Brivio wrote:
> 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.

It is, yes.  Sorry, forgot to include that.

> > ---
> >  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
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]