From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 000065A026A for ; Mon, 13 Feb 2023 04:12:07 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4PFTsG04Nnz4x82; Mon, 13 Feb 2023 14:12:02 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1676257922; bh=NDekzNZViGqStboy9B6d3BhwzJqp7aKQg8sBH3CIcaY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZGtpudQ85Z86k/jBMOXqY99up4iTn7M2EIo2Nl5l6c11ld9nJ/95GJTT2GRa3VdmV ECEAEySax775uJWb1VaFcgIM7UhDk926jxuBG0GaoHdvlXyKsj2Ansmkc6gO7GHnai bHIeIGbRHDVgTPP6C9sFHeYni2mP2Iy/qHo+6MGk= Date: Mon, 13 Feb 2023 13:33:45 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] pasta: Wait for tap to be set up before spawning command Message-ID: References: <20230213011319.1198880-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ZTMGUZhxRpWOVEG6" Content-Disposition: inline In-Reply-To: <20230213011319.1198880-1-sbrivio@redhat.com> Message-ID-Hash: HYLZH4LQOC3AKY6FJLWXAQP5QKSBLCLJ X-Message-ID-Hash: HYLZH4LQOC3AKY6FJLWXAQP5QKSBLCLJ 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: passt-dev@passt.top, Paul Holzinger X-Mailman-Version: 3.3.3 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: --ZTMGUZhxRpWOVEG6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 13, 2023 at 02:13:19AM +0100, Stefano Brivio wrote: > Adapted from a patch by Paul Holzinger: when pasta spawns a command, > operating without a pre-existing user and network namespace, it needs > to wait for the tap device to be configured and its handler ready, > before the command is actually executed. >=20 > Otherwise, something like: > pasta --config-net nslookup passt.top >=20 > usually fails as the nslookup command is issued before the network > interface is ready. >=20 > We can't adopt a simpler approach based on SIGSTOP and SIGCONT here: > the child runs in a separate PID namespace, so it can't send SIGSTOP > to itself as the kernel sees the child as init process and blocks > the delivery of the signal. >=20 > We could send SIGSTOP from the parent, but this wouldn't avoid the > possible condition where the child isn't ready to wait for it when > the parent sends it, also raised by Paul -- and SIGSTOP can't be > blocked, so it can never be pending. >=20 > Use SIGUSR1 instead: mask it before clone(), so that the child starts > with it blocked, and can safely wait for it. Once the parent is > ready, it sends SIGUSR1 to the child. If SIGUSR1 is sent before the > child is waiting for it, the kernel will queue it for us, because > it's blocked. >=20 > Reported-by: Paul Holzinger > Fixes: 1392bc5ca002 ("Allow pasta to take a command to execute") > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > I just applied this, to keep things simpler for myself and for > tests, given that Paul already had a look. I'm posting it anyway > for reviews. >=20 > passt.c | 3 +++ > pasta.c | 14 +++++++++++++- > pasta.h | 2 ++ > 3 files changed, 18 insertions(+), 1 deletion(-) >=20 > diff --git a/passt.c b/passt.c > index 8b2c50d..d957e14 100644 > --- a/passt.c > +++ b/passt.c > @@ -301,6 +301,9 @@ int main(int argc, char **argv) > else > write_pidfile(pidfile_fd, getpid()); > =20 > + if (pasta_child_pid) > + kill(pasta_child_pid, SIGUSR1); > + > isolate_postfork(&c); > =20 > timer_init(&c, &now); > diff --git a/pasta.c b/pasta.c > index 528f02a..9169913 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -47,7 +47,7 @@ > #include "log.h" > =20 > /* PID of child, in case we created a namespace */ > -static int pasta_child_pid; > +int pasta_child_pid; > =20 > /** > * pasta_child_handler() - Exit once shell exits (if we started it), rea= p clones > @@ -166,10 +166,16 @@ struct pasta_spawn_cmd_arg { > static int pasta_spawn_cmd(void *arg) > { > const struct pasta_spawn_cmd_arg *a; > + sigset_t set; > =20 > if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0")) > warn("Cannot set ping_group_range, ICMP requests might fail"); > =20 > + /* Wait for the parent to be ready: see main() */ > + sigemptyset(&set); > + sigaddset(&set, SIGUSR1); > + sigwaitinfo(&set, NULL); > + > a =3D (const struct pasta_spawn_cmd_arg *)arg; > execvp(a->exe, a->argv); > =20 > @@ -196,6 +202,7 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t g= id, > char ns_fn_stack[NS_FN_STACK_SIZE]; > char *sh_argv[] =3D { NULL, NULL }; > char sh_arg0[PATH_MAX + 1]; > + sigset_t set; > =20 > c->foreground =3D 1; > if (!c->debug) > @@ -226,6 +233,11 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t = gid, > arg.argv =3D sh_argv; > } > =20 > + /* Block SIGUSR1 in child, we queue it in main() when we're ready */ > + sigemptyset(&set); > + sigaddset(&set, SIGUSR1); > + sigprocmask(SIG_BLOCK, &set, NULL); > + > pasta_child_pid =3D do_clone(pasta_spawn_cmd, ns_fn_stack, > sizeof(ns_fn_stack), > CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET | > diff --git a/pasta.h b/pasta.h > index a8b9893..0ccb7e9 100644 > --- a/pasta.h > +++ b/pasta.h > @@ -6,6 +6,8 @@ > #ifndef PASTA_H > #define PASTA_H > =20 > +extern int pasta_child_pid; > + > void pasta_open_ns(struct ctx *c, const char *netns); > void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, > int argc, char *argv[]); --=20 David Gibson | 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 --ZTMGUZhxRpWOVEG6 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmPpoYMACgkQzQJF27ox 2GcFmBAAiHjh/qJ2Xz3C+UUXN2u1FUZnTz88bzUlXZ1xHymao10AMtWIZNnbmBIt aJmNdet3zdygCGW7EOKG0xG6sWeoxZL2+T40Du0H0gExZ1ob6T4zvhRBtNsfUdt2 PL09zNYZ86kTAWOIjV0pTywZV/i9x1f7nG7XxhVCgR1A0Ryu1INTJfJg+WxB7Hb4 dJ9LZcTQcp9D/INEnKMFKSOo2MHwZpRKHTGys/61tkempzHFWTEnJHEMJptcblIG wIXIpIGchqOsk1DFaDG5GpgnANPsj+smgq+d0cvZNaaQzEWI6aSE0rRyuCAHroNV EPgvRrg0136FmGTkafbZm1TsWl5ui74IkYZymZJtkYCVRJacRu/nJaR75uHxVvwN GR8cruQrDGSQIoNCiBBp4DCVP0mfqqVgdiL/FAokic/ddS1Uf9rJNTkQaG/Ln7+x lqkcRLvLNOb5yZYew+ui0TyY8RUp/G7ZMiL7p1m5S1gmvLEXpWOx7IIW6AsIZYiE CWBfiDSjUw8edfk9ZBiTwDCIp/FS2jcNXoJGNqof69GsivpPqEb0r+ykAKQ2UqVH o0DA4b21vBH4IYfDDkFwjhNyZ+49mBwBP7znSKzxtvawELN70f2EM58p2aSxg3nn +8Bl2/GQFYOw+/+w6WEA1J7PSS8QyEpxcx3zU0ifNLamF37j4Cc= =1StZ -----END PGP SIGNATURE----- --ZTMGUZhxRpWOVEG6--