From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202512 header.b=dypviN7U; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id AE1525A0271 for ; Thu, 11 Dec 2025 00:45:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1765410348; bh=SfAXu92RMHJH5Hly7XXM5m7gzsfcnSuLDCuAODgrtRU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dypviN7UefeOowlwUQQld6PG4CS3Ch6sHYEbRjoTsc9KtWH4kmL4Wm23TLXhvR14Z hgdNA9HU7r4OMWp7T3Qa4PT5Lg9JP3CAt0NC9SKmnEodVPHO19qU9EvDZ37ZchZHDU bnPBLQeZwgl+0yXV2RFBSQP0MZv7/t+GOwuV+yNCB+1EhxG0gUks7tGO2hZyf8DwZE d1jTaflhd0RdJTCdJPi4f5Tyt22YBsNKTIFuqacyR8lBhkxbWJ/uwrp/TmIcKzfQUr yv8PdGqGXEUrxc1BHpflLX5mlFn0TBtKG49kPlh70iuKHK3+ok50B9MoTPw7J3olGr JCJx8z6ZnH6zw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dRXR02sYNz4wM3; Thu, 11 Dec 2025 10:45:48 +1100 (AEDT) Date: Thu, 11 Dec 2025 10:45:14 +1100 From: David Gibson To: Paul Holzinger Subject: Re: [PATCH 2/2] pasta: Clean up waiting pasta child on failures Message-ID: References: <20251210051552.1157177-1-david@gibson.dropbear.id.au> <20251210051552.1157177-3-david@gibson.dropbear.id.au> <3b553775-1ee8-4b87-8b9c-ff0dec281576@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Fo/2RybUxMtDnJZ1" Content-Disposition: inline In-Reply-To: <3b553775-1ee8-4b87-8b9c-ff0dec281576@redhat.com> Message-ID-Hash: REIC7DZTOGOR4WIKYIMM6YVQ27LBPTA5 X-Message-ID-Hash: REIC7DZTOGOR4WIKYIMM6YVQ27LBPTA5 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Stefano Brivio , passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Fo/2RybUxMtDnJZ1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 10, 2025 at 01:05:10PM +0100, Paul Holzinger wrote: >=20 > On 10/12/2025 06:15, 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. > >=20 > > 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. > >=20 > > Make this cleaner, by having passt_exit() terminate the child, when cal= led > > during this window. > >=20 > > Signed-off-by: David Gibson > > --- > > passt.c | 1 + > > pasta.c | 2 ++ > > pasta.h | 1 + > > util.c | 5 +++++ > > 4 files changed, 9 insertions(+) > >=20 > > diff --git a/passt.c b/passt.c > > index cf38822f..955c7091 100644 > > --- a/passt.c > > +++ b/passt.c > > @@ -431,6 +431,7 @@ int main(int argc, char **argv) > > if (pasta_child_pid) { > > kill(pasta_child_pid, SIGUSR1); > > log_stderr =3D false; > > + pasta_child_signalled =3D true; > > } > > isolate_postfork(&c); > > diff --git a/pasta.c b/pasta.c > > index 5c693de1..8ac4511f 100644 > > --- a/pasta.c > > +++ b/pasta.c > > @@ -54,6 +54,8 @@ > > /* PID of child, in case we created a namespace */ > > int pasta_child_pid; > > +/* Has the child been signalled to start a shell or command */ > > +bool pasta_child_signalled; > > /** > > * pasta_child_handler() - Exit once shell exits (if we started it), = reap clones > > diff --git a/pasta.h b/pasta.h > > index 4b063d13..55028c74 100644 > > --- a/pasta.h > > +++ b/pasta.h > > @@ -7,6 +7,7 @@ > > #define PASTA_H > > extern int pasta_child_pid; > > +extern bool pasta_child_signalled; > > void pasta_open_ns(struct ctx *c, const char *netns); > > void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > > diff --git a/util.c b/util.c > > index e266c396..4744f09e 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 > > #endif > > @@ -1235,6 +1236,10 @@ void abort_with_msg(const char *fmt, ...) > > */ > > void passt_exit(int status) > > { > > + /* If we're starting our own namespace, don't leave it in limbo */ > > + if (pasta_child_pid && !pasta_child_signalled) > > + kill(pasta_child_pid, SIGTERM); >=20 > Any reason not to use SIGKILL? Then there is no doubt if it might be igno= red > or not. Eh, only that some log might show KILLs on the assumption that they indicate a bad thing happening, which isn't really the case here. > Another thing I assume the goal here is to only kill the process if we > didn't exec yet. Yes, that was the goal. > I wonder how much value there is to have the child process > outlive pasta. That's a good question, and I don't have a strong opinion either way. I leant this way based on two factors: - If this happens later, once the child is established, it's possible the user could have started running something there that remains valuable even if it loses its network - If pasta dies once the child has exec()ed, the symptoms are both more obvious and easier to clean up: the shell (or whatever) is right there, running, and can be exited in the normal way (e.g. running to completion or ^C). Before the exec() this leaves a process running non-obviously which has to be spotted and killed explicitly by the user. (also note that if we do kill the child after exec(), we definitely want to use SIGTERM not SIGKILL to allow it to clean up gracefully). > I feel like using something like PR_SET_PDEATHSIG might be a > safer option in case pasta crashes without going through passt_exit. Oh, good idea. I forgot PR_SET_PDEATHSIG was a thing. > And if > you don't want to propagate that into the user command we could unset it > right again before the exec as well. >=20 > > + > > /* Make sure we don't leave the pcap file truncated */ > > if (pcap_fd !=3D -1 && fsync(pcap_fd)) > > warn_perror("Failed to flush pcap file, it might be truncated"); >=20 > --=20 > Paul Holzinger >=20 --=20 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 --Fo/2RybUxMtDnJZ1 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmk6BgkACgkQzQJF27ox 2GeCRQ//VZobx/YbnTf9Ewuz+nU0C8acMHWqwQXlhsF8sSNPxOYTFHr2C28d6GbQ i8geFNzhX5gt7uVrGtajNl0hP96p83dvjFHiChregQJ8Ugc/3A8ChahCn/x1AV9d Ep+x6aVAtz6EEaJrp7fh1Sk4qIvwUW4EP5RecrsdBkRWABQMbDlJiTrS/toa6Rjd RhTAwmYckRaPnQhbvYCQTkrKpztJUCGBfN4VZOCNEA2Q8/ATIa8avqXNild4b5Oo VmMhWluWZT7jZUGTENPiALl+lWY+Y2w5xspB95IKX9cRn+1AK5O8MhatKkgNCtaB +67LRjgv5NDKliAB+EivdVvBzeyTs8rbkklzI2uavZ0exNqmgahwHdyQjZUGd2vQ GqZ7LoIiN8BHCj2eyPia30ZSfSPI8arr48//hkjjKik7sj4xjJdYfHySH/RzwbIc OV3NPKXYNw159Xj+NzklIWmpJ5Bg6X30pSkGizIjg90oPtqN+gXYzwFD4iHK2Je1 sdYxEAXunSHtVIyaLgpHGpJFK61pp4i/idcE7JJtWqRLVmyv8knHTRVYpUK5Ifi2 sdAFfuTHsenx1JPPtJHyD/mIsJ2OPmuFBkVl/tEPoIqu2o9DtNp28/WyRqgLkojg usA2T00jm+oKx4lSXN1wf0madAE8KFPR08/Rmlgu4Qkt8cuR9RI= =dt1e -----END PGP SIGNATURE----- --Fo/2RybUxMtDnJZ1--