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=202602 header.b=CijcekVk; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C22185A0262 for ; Mon, 18 May 2026 04:54:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1779072890; bh=bgDaYrMJpirBRhMWfvpuGU9Ysjw0LLBNsQbeFTjmoog=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=CijcekVk1oG/6KIJNVOav9xUmyMb+Djs6a6+1wvXQ1teMMs3x+0Hol/+rOGSs2IAQ No8EkJo+Uu4zL1skxTMNR75Fhvqov4dDmqjsMPSi2E3E6+oJdqAXNKwYF+d1YGioA2 woAwvqckCyQ0wZvMQHlz3BbCjktUpj4AMhofl4TIh09plxZI4DpX1RUBE7vDEXN2HK XXaVmIzytmJfeduxuX83JzhJ7tkymEgTuEJm6n744woZtbFtBNMXIdrB29cvowUvjC JHGNMM83TMHTtxg7Ka16V3O7I97Ohy3/YQFeesiiE4/i5te5ss7VEv0xODPgALFXPm +LRAKEtgBep+Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gJj8B3yQRz4wT1; Mon, 18 May 2026 12:54:50 +1000 (AEST) Date: Mon, 18 May 2026 12:28:57 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] treewide: Add SOCK_CLOEXEC to accept() calls that are missing it Message-ID: References: <20260513041423.2446716-1-david@gibson.dropbear.id.au> <20260513041423.2446716-2-david@gibson.dropbear.id.au> <20260516174610.3ee899b5@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Y4s1UAm0FMUcYoX1" Content-Disposition: inline In-Reply-To: <20260516174610.3ee899b5@elisabeth> Message-ID-Hash: 53GMGOX2LC4WZUMZW75S3DACS5OWUFBS X-Message-ID-Hash: 53GMGOX2LC4WZUMZW75S3DACS5OWUFBS 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 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: --Y4s1UAm0FMUcYoX1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, May 16, 2026 at 05:46:11PM +0200, Stefano Brivio wrote: > On Wed, 13 May 2026 14:14:21 +1000 > David Gibson wrote: >=20 > > Generally we try to set the O_CLOEXEC flag on every fd we create. This > > seems to be generally accepted security best practice these days, and we > > never fork(), so certainly have no need to pass fds to children. >=20 > But we do clone() with CLONE_FILES (even though when we clone() to call > execvp() later, we don't set CLONE_FILES), so, even though I don't see > a reason to skip O_CLOEXEC for c->fd_tap, this conclusion shouldn't be > automatic from the fact we don't fork(). So, I did think about that when wrote it, but went for the short version rather than saying clone() with CLONE_FILES doesn't count. Now, I realised that we've both fallen for the trap again, forgetting that this has nothing to do with fork() or clone() and is, as it says right there in the name, about exec(). I'll update the message. > I spent some time on it and I really couldn't find a reason why we > don't have O_CLOEXEC there, so probably there isn't any, and I think > this patch is fine. >=20 > I would just change this paragraph to "[...] these days, and we don't > need to pass file descriptors to children." >=20 > > A handful of accept4() calls on Unix sockets are missing the SOCK_CLOEX= EC > > flag to set this though. Add the missing flag. > >=20 > > Signed-off-by: David Gibson > > --- > > repair.c | 5 +++-- > > tap.c | 4 ++-- > > 2 files changed, 5 insertions(+), 4 deletions(-) > >=20 > > diff --git a/repair.c b/repair.c > > index 69c53077..3e0e3e0a 100644 > > --- a/repair.c > > +++ b/repair.c > > @@ -87,7 +87,7 @@ int repair_listen_handler(struct ctx *c, uint32_t eve= nts) > > /* Another client is already connected: accept and close right away. = */ > > if (c->fd_repair !=3D -1) { > > int discard =3D accept4(c->fd_repair_listen, NULL, NULL, > > - SOCK_NONBLOCK); > > + SOCK_NONBLOCK | SOCK_CLOEXEC); > > =20 > > if (discard =3D=3D -1) > > return errno; > > @@ -99,7 +99,8 @@ int repair_listen_handler(struct ctx *c, uint32_t eve= nts) > > return EEXIST; > > } > > =20 > > - if ((c->fd_repair =3D accept4(c->fd_repair_listen, NULL, NULL, 0)) < = 0) { > > + if ((c->fd_repair =3D accept4(c->fd_repair_listen, NULL, NULL, > > + SOCK_CLOEXEC)) < 0) { > > rc =3D errno; > > debug_perror("accept4() on TCP_REPAIR helper listening socket"); > > return rc; > > diff --git a/tap.c b/tap.c > > index 0920a325..e7cac9df 100644 > > --- a/tap.c > > +++ b/tap.c > > @@ -1477,7 +1477,7 @@ void tap_listen_handler(struct ctx *c, uint32_t e= vents) > > /* Another client is already connected: accept and close right away. = */ > > if (c->fd_tap !=3D -1) { > > int discard =3D accept4(c->fd_tap_listen, NULL, NULL, > > - SOCK_NONBLOCK); > > + SOCK_NONBLOCK | SOCK_CLOEXEC); > > =20 > > if (discard =3D=3D -1) > > return; > > @@ -1490,7 +1490,7 @@ void tap_listen_handler(struct ctx *c, uint32_t e= vents) > > return; > > } > > =20 > > - c->fd_tap =3D accept4(c->fd_tap_listen, NULL, NULL, 0); > > + c->fd_tap =3D accept4(c->fd_tap_listen, NULL, NULL, SOCK_CLOEXEC); > > =20 > > if (!getsockopt(c->fd_tap, SOL_SOCKET, SO_PEERCRED, &ucred, &len)) > > info("accepted connection from PID %i", ucred.pid); >=20 > --=20 > Stefano >=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 --Y4s1UAm0FMUcYoX1 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoKeWgACgkQzQJF27ox 2GdU7g/+OkA2P5cQZ9Fyk1jkNyZbnVaGXIrpUT4Uk2NUB21lTaQAN49Of7MPoreI oQQ8m01IE6VyXPzfJyk6q4MAh93piaqQupuV0ou36RdrsA+0YmfN7L+dJfdcKdHH bguEnyCsZ2x/2wXuemZsTLlbiSJoO3/Ih4RXRQSXdBv0OaSzqrLZ5i5oZ71iCkLF wV6ukTwXCgiGD5VO45169JwIAGYSdaxbFRWUierErcOyOK7j7yvGF5qSptyHBfrC IhzAIsp1EevULb0H+K0fNq7AZG9i7RyQdhTMtkErkTdXBFJ1pv+Fce5bIlN+zS4m 8cfXAHwP8OFg89HBphb52nO4ECY+3/KalpU0UowA245rYzzxWyf9wEjeUbxRCB8K 9BSW2dUSNUIwEyhAluufimcfl+J94xpwGT/ZaR32bI60u+9u4WKfM0/HmHfDEonV fZDRxfzuNxEJhFZ79En99UQ2M+0X3atPCukSD0N9iSWkO8SaYawy2T9C/xDcPS1t Iyn+g6WrDxDhWPPPi1H/40FKgfGVqW3xFmGX0d5r1VwZrrdkmwVK0a8y5Sni4DKZ BtNhFfD2bGEdxHlDUNCjHj/p+q9vb+6kFjw0qhPcuhll7gyNsb0MAZkLJqzoKhR8 kxqTvjC2s63pMphwcG7NgkLEGAnYcMwnpq7jia3UKkYo/lXw7Jk= =0eTX -----END PGP SIGNATURE----- --Y4s1UAm0FMUcYoX1--