On Wed, Dec 11, 2024 at 01:46:41AM +0100, Stefano Brivio wrote:On Wed, 11 Dec 2024 11:26:46 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Fair point.On Wed, Dec 11, 2024 at 12:29:09AM +0100, Stefano Brivio wrote: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().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. 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). 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). Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/24804 Analysed-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>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.Ok.Otherwise I guess 'strerror_()' or 'passt_strerror()', ugly as those are.Picked strerror_(), passt_strerror() is way too long.True.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?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.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. -- 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