On Wed, 6 Nov 2024 17:54:17 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote: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 ifdefsFor 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.* Even if it did fix the compile, it means inconsistent behaviour between a compile time failure to find the value (we silently don't close files) and a runtime failure (we die with an error from close_range())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"). 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.* Silently not closing the files we intend to close for security reasons is probably not a good idea in any caseIt's arguably even worse to force users to run containers or guests as root. That's the reason for the no-op implementation. I don't think that introducing a dependency on a >= 5.9 kernel is a good idea. 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.As bonus this fixes a cppcheck error I see with some different options I'm looking to apply in future. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- linux_dep.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 { #include <linux/close_range.h> -#ifdef CLOSE_RANGE_UNSHARE /* Linux kernel >= 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 #endif /* LINUX_DEP_H */-- Stefano