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=202410 header.b=JtFx9gUj; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 484785A061B for ; Wed, 06 Nov 2024 22:16:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1730927782; bh=Px0nP4PP/o3t64Cl9XDDpmZ/gmPPdllVmWITeq+89hc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JtFx9gUjWA5pJj75/YE69KeQLrPTbi3S5kRczmDokzTwlarWGJuHchiQm3pTvInot bniCWknPkrEbnQNytUXlABFGBcK6ezbkJ0utRHDMGiMAdN467fuRKUPVTsJa+cQzPg xFc5MuCMFsuTyU79xOWO9eNVYCJBFzuwHcEtRNoIXiiHwlZUUhhZ4T63IWmncakiTH CzhY6m7oyuLIJM9KdmAu6msOaU2cqWEOS5CCZNNEhCSV1FL24wnYyFMLdOm8ugGBHG 110+tuXAD6XgJYt2gfJ7qXDE3lLD9Br2Qa8Qe8lCBbRlwAN/ih394V2hmd/N4ILzsB XabOdAsuNoEDw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4XkJ0k72lqz4x9G; Thu, 7 Nov 2024 08:16:22 +1100 (AEDT) Date: Thu, 7 Nov 2024 08:01:11 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 4/8] linux_dep: Fix CLOSE_RANGE_UNSHARE availability handling Message-ID: References: <20241106065421.2568179-1-david@gibson.dropbear.id.au> <20241106065421.2568179-5-david@gibson.dropbear.id.au> <20241106201223.7e45f87d@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OpER0zeiJeLRANiI" Content-Disposition: inline In-Reply-To: <20241106201223.7e45f87d@elisabeth> Message-ID-Hash: QDWD7O5WP5HQKXXVCA4HWEGCZBLYLJ47 X-Message-ID-Hash: QDWD7O5WP5HQKXXVCA4HWEGCZBLYLJ47 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: --OpER0zeiJeLRANiI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 06, 2024 at 08:12:23PM +0100, Stefano Brivio wrote: > On Wed, 6 Nov 2024 17:54:17 +1100 > David Gibson wrote: >=20 > > If CLOSE_RANGE_UNSHARE isn't defined, we define a fallback version of > > close_range() which is a (successful) no-op. This is broken in several > > ways: > > * It doesn't actually fix compile if using old kernel headers, because > > the caller of close_range() still directly uses CLOSE_RANGE_UNSHARE > > unprotected by ifdefs >=20 > For users using distribution packages, that is, pretty much everybody > not developing passt, this is generally not a concern, because build > environments typically ship kernel headers matching the C library and > the distribution version at hand. Unlike some of the other things fixed recently, this one is not related to compile time versus runtime checks. This one is simply broken compiling against an older kernel, regardless of the runtime version. Without this patch, close_open_files() directly uses CLOSE_RANGE_UNSHARE unprotected by an #ifdef. > > * Even if it did fix the compile, it means inconsistent behaviour betw= een > > a compile time failure to find the value (we silently don't close fi= les) > > and a runtime failure (we die with an error from close_range()) >=20 > Given that this is mostly relevant for stuff built against musl (and > running on a musl-based distribution), that's not really a problem in > practice. See 6e9ecf57410b ("util: Provide own version of close_range(), > and no-op fallback"). >=20 > But sure, I'm fine with these changes in general, as they're strictly > speaking more correct than the current behaviour, minus the next point. >=20 > > * Silently not closing the files we intend to close for security reaso= ns > > is probably not a good idea in any case >=20 > It's arguably even worse to force users to run containers or guests as > root. That's the reason for the no-op implementation. Uh... what's the connection to running as root? > I don't think that introducing a dependency on a >=3D 5.9 kernel is a > good idea. We already have a dependency on compiling against a >=3D 5.9 kernel, see above. > If this bothers you or cppcheck, I'd rather call close_range() > (possibly the weakly aliased implementation), and log a warning on > ENOSYS instead of failing. We could do that. We'd need to consider EINVAL as well as ENOSYS, for the case that the syscall exists, but the CLOSE_RANGE_UNSHARE flag isn't recognized. > > As bonus this fixes a cppcheck error I see with some different options = I'm > > looking to apply in future. > >=20 > > Signed-off-by: David Gibson > > --- > > linux_dep.h | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > >=20 > > diff --git a/linux_dep.h b/linux_dep.h > > index 3a41e42..240f50a 100644 > > --- a/linux_dep.h > > +++ b/linux_dep.h > > @@ -127,22 +127,18 @@ struct tcp_info_linux { > > =20 > > #include > > =20 > > -#ifdef CLOSE_RANGE_UNSHARE /* Linux kernel >=3D 5.9 */ > > /* glibc < 2.34 and musl as of 1.2.5 need these */ > > #ifndef SYS_close_range > > #define SYS_close_range 436 > > #endif > > +#ifndef CLOSE_RANGE_UNSHARE /* Linux kernel < 5.9 */ > > +#define CLOSE_RANGE_UNSHARE (1U << 1) > > +#endif > > + > > __attribute__ ((weak)) > > /* cppcheck-suppress funcArgNamesDifferent */ > > int close_range(unsigned int first, unsigned int last, int flags) { > > return syscall(SYS_close_range, first, last, flags); > > } > > -#else > > -/* No reasonable fallback option */ > > -/* cppcheck-suppress funcArgNamesDifferent */ > > -int close_range(unsigned int first, unsigned int last, int flags) { > > - return 0; > > -} > > -#endif > > =20 > > #endif /* LINUX_DEP_H */ >=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 --OpER0zeiJeLRANiI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmcr2RYACgkQzQJF27ox 2GfhrQ/6A8S3tsnis+DvHeGa9svIjIZGazJ6HqHlKmoT6EUfZg20zEWMWAOxDJ5S ytxWxmNfbgd02K/8IYMHJwgtU7CIcZq7ScJvjmgRzrEDYzPWQW/PyGT3xdGCuZst ravbmZMeTYUiV7DVUJbnVrAM6Oiv0utkNrT8CHeUk4GlBudjRm5qJOUP/3x8GC5A 0yskjIaH4BjB3bbp9PX8EODIv76IVth5ROABsao9eP0VgLoKNJY3Ta3rU5IFVAwm l26b68xeGMGEUQdo4HTl9/QOC/E9PGfaP9GoI+cX1MzpTgsb/jI1Z8oe69aPd3Hg X+j710t2Fy8SB5xf+XmMm+zI86hxjY2Oi15I8XtvzfLAOZKIJnwyz2hx/HCS539e pH0ImdnNCMwQGJcmb9lHljqiMalcvs8V02xoUoRUTLDsThz9jloquCQOEiAdeoNW KZFChsbbNs61XPgXAtSTTIbHoObcrdlB3LHHusKOAYIkJFzY38xL80Ze5VTeZQje kwIDskqZuRWrcFkTHqVhZuPVa+MKAfjpCiCho4MBewYGFcUEPm2RtonNaH61WQFU 2taEfzOWen7CoI1sTet29lcZanHgxSbMl2CA/WGh7D+sjTfwAgY9Q+tZhAFD/+yg /zSRxW0U1FO3Roh3WZQFKIjhJwlXo6j0P23NNgW8rnZUOTMzGR8= =s1GL -----END PGP SIGNATURE----- --OpER0zeiJeLRANiI--