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=202412 header.b=V+XGJ5XW; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 81A8B5A0274 for ; Wed, 11 Dec 2024 04:55:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1733889350; bh=T8kUpSAddMlJPWdhgCmojb0B/xf5CEDlg56LZINtGDI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V+XGJ5XWe8rH1YqY93uNb9bdDft8ohdwSutWcOE1Wr6cj/FpEbtMNVhwHglxlgzxi o146VsPVVgZRlyaQcwzC6cJgqqxqm1XW797xnnh8+nkXJX11AVOc4/yUnIxOPpsxdY /1UUYLXKTnci+EO+NH9D+fzMv3Mqs71rbQBpj2dKFgAVmHY1WXprYlP48C7B+S+8ls TVEVwARFDsirBbYduhM42uwTDxOQTEnasFibeRMS4RoMgqlehVxJ+ZTStDJ3NnLjgD BHiG8qiR5O9ngn02sJPSeWNnlllgcuESD1tY3GR13ttyl/RLWYvxcjKorXKbMs7QOr t8A/y2vVuxB+g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Y7MFy1yLNz4wp0; Wed, 11 Dec 2024 14:55:50 +1100 (AEDT) Date: Wed, 11 Dec 2024 14:55:42 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] treewide: Dodge dynamic memory allocation in strerror() from glibc > 2.40 Message-ID: References: <20241210232909.1034903-1-sbrivio@redhat.com> <20241211014641.27e438ca@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4JNuCq/HGbOE/9J3" Content-Disposition: inline In-Reply-To: <20241211014641.27e438ca@elisabeth> Message-ID-Hash: NQ2Z6CJCYOTT3SLAVQZNGAB2KSHK35GS X-Message-ID-Hash: NQ2Z6CJCYOTT3SLAVQZNGAB2KSHK35GS 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.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: --4JNuCq/HGbOE/9J3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 11, 2024 at 01:46:41AM +0100, Stefano Brivio wrote: > On Wed, 11 Dec 2024 11:26:46 +1100 > David Gibson wrote: >=20 > > On Wed, Dec 11, 2024 at 12:29:09AM +0100, Stefano Brivio wrote: > > > With glibc commit 25a5eb4010df ("string: strerror, strsignal cannot > > > use buffer after dlmopen (bug 32026)"), strerror() now needs, at least > > > on x86, the getrandom() and brk() system calls, in order to fill in > > > the locale-translated error message. But getrandom() and brk() are not > > > allowed by our seccomp profiles. > > >=20 > > > This became visible on Fedora Rawhide with the "podman login and > > > logout" Podman tests, defined at test/e2e/login_logout_test.go in the > > > Podman source tree, where pasta would terminate upon printing error > > > descriptions (at least the ones related to the SO_ERROR queue for > > > spliced connections). > > >=20 > > > Avoid dynamic memory allocation by calling strerrordesc_np() instead, > > > which is a GNU function returning a static, untranslated version of > > > the error description. If it's not available, keep calling strerror(), > > > which at that point should be simple enough as to be usable (at least, > > > that's currently the case for musl). > > >=20 > > > Reported-by: Paul Holzinger > > > Link: https://github.com/containers/podman/issues/24804 > > > Analysed-by: Paul Holzinger > > > Signed-off-by: Stefano Brivio =20 > >=20 > > In the short term, this seems like a reasonable fix, except for one > > detail. Generally '_' prefixed names are reserved for "the system", > > meaning libc in practice. So I don't think __strerror() is a good > > choice for our interposed function name. If we can keep the interface > > the same and use macro shannigans to interpose it would be nice if we > > can keep it as 'strerror()' through most of the code. I think this is > > possible - see the clang tidy workarounds at the bottom of util.h for > > example. >=20 > It definitely is, it's even simpler than those as we wouldn't even need > local variables here, but... it's not strerror(), so I don't think we > should call it strerror(). Fair point. >=20 > > Otherwise I guess 'strerror_()' or 'passt_strerror()', ugly > > as those are. >=20 > Picked strerror_(), passt_strerror() is way too long. Ok. > > Longer term, it's not awesome to be ignoring the locale. Could we use > > the XSI compliant strerror_r() version to get translated messages > > somewhat portably and without allocation? >=20 > I gave that a thought, but the XSI compliant version of strerror_r() > needs a user-supplied buffer as well, which would make the callers look > horrible. We could pass in a static buffer otherwise, but then, > confusingly, our own version/wrapper of strerror_r() wouldn't be > thread-safe. True. > I wonder if ignoring the locale is really that bad. After all, we print > all the messages in English, without localisation. Printing the error > description in other languages is arguably inconsistent. Hm, I guess. Maybe we can tackle respecting locale for the kernel library errors when/if we add localisation support for our own messages. --=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 --4JNuCq/HGbOE/9J3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmdZDSMACgkQzQJF27ox 2GdvWBAAjs9bFbKTrpGDTMOjUikZ+v7QmFexEX2Z9/5xBlZbdA0WV+HanDvGin5C R8/ERwlX+ZLBvMogD6RaFvqVEraulGlcdrq73Nw2scw/f58tfJU9rqg2Zfo+FxnI fg/mwguN5i7xXd5fkPMHflwFan9yVtAEks3m7xNYTk4ZPdbehm69yZcu2jQuNF+r WwGBobOoEPlTNmaLHmr0BUyxedA0eqtQCzxx5oIE5BNz2pncNIw/8hqzZU1o1oZL BidTxRX59qw4D881CqW7G2tyNwjFlP8rrJhUCJR0vXa5EIph4OROSMPbEx5bVL39 f1/OSo7QAdeCGnJqN4/7Z9G73f4iwO6It6wJ+ITtsQfc1QplRnLKSoBKIS6JYU2h AC74mFM+wKZcT+TK83X3UMp+eY1KvjMR6k1ttjxWEUO9UUvLUhjLsaQZqEEqDbZl A4Q4Qjr/JRRdlJMLaLDpkIJAAMtlHtIyOomnfeoj2g8ZBTPMxRVjLONGRG9ivagV 4IOFwpu0tNKplxXd3uLCDqpIx5adTMZGpUnFWly2vQ192zdjaTih197Xhky7FuWo 7WuS0sRjLQwel3ZkUGeSHn2FV92dfTUiSuFRxQERvUrtqqGbEZORD6+wovqIDJ4C QLaP8olctS6Ka1/dvpfMol7GN+60BRPUCkaQW7MJDSdUlGlRZFk= =J5FW -----END PGP SIGNATURE----- --4JNuCq/HGbOE/9J3--