Re: [musl] [PATCH v2] add close_range() syscall wrapper

[Rich Felker <dalias@libc.org>]

On Thu, Aug 01, 2024 at 12:43:00PM +0300, Alexey Izbyshev wrote:
> On 2023-09-01 17:58, Natanael Copa wrote:
> >close_range() is a syscall present in FreeBSD 8.0 and Linux 5.9. glibc
> >2.34 added a wrapper.
> >
> >Expose it under _GNU_SOURCE similar to what GNU libc does. Also expose
> >it under _BSD_SOURCE since it is also a FreeBSD function.
> >---
> >
> >v2: use syscall without __syscall_ret
> >
> > include/unistd.h        | 3 +++
> > src/linux/close_range.c | 8 ++++++++
> > 2 files changed, 11 insertions(+)
> > create mode 100644 src/linux/close_range.c
> >
> >diff --git a/include/unistd.h b/include/unistd.h
> >index 5bc7f798..d89e3d4c 100644
> >--- a/include/unistd.h
> >+++ b/include/unistd.h
> >@@ -161,6 +161,9 @@ unsigned ualarm(unsigned, unsigned);
> > #define L_INCR 1
> > #define L_XTND 2
> > int brk(void *);
> >+#define CLOSE_RANGE_UNSHARE	(1U << 1)
> >+#define CLOSE_RANGE_CLOEXEC	(1U << 2)
> >+int close_range(unsigned int, unsigned int, int);
> > void *sbrk(intptr_t);
> > pid_t vfork(void);
> > int vhangup(void);
> >diff --git a/src/linux/close_range.c b/src/linux/close_range.c
> >new file mode 100644
> >index 00000000..3f1378a0
> >--- /dev/null
> >+++ b/src/linux/close_range.c
> >@@ -0,0 +1,8 @@
> >+#define _GNU_SOURCE
> >+#include <unistd.h>
> >+#include "syscall.h"
> >+
> >+int close_range(unsigned int first, unsigned int last, int flags)
> >+{
> >+	return syscall(SYS_close_range, first, last, flags);
> >+}
> 
> Regarding FreeBSD, close_range was added not in 8.0, but in 13.0
> [1], and also backported to 12.2 [2].
> 
> Otherwise, this patch looks good to me.
> 
> Rich, is it possible to consider close_range wrapper inclusion
> again? Apart from FreeBSD and glibc, bionic has it too. A cursory
> Debian code search shows that close_range libc wrapper can be used
> at least by openssh, libvirt, network-manager, openrc, qemu, lxc,
> rsyslog packages (in addition to CPython that I mentioned ealier).
> 
> As for having a fallback in case the syscall is unavailable, I'm not
> aware of anybody implementing it, so I'd expect all close_range
> users to implement their own fallback/error handling. For example,
> Debian's openssh migrated from closefrom to close_range with their
> own fallback because of too aggressive closefrom fallback in
> glibc[3].
> 
> Thanks,
> Alexey
> 
> [1] https://cgit.freebsd.org/src/commit/?h=releng/13.0&id=472ced39efb537374068f06b348fe5dac389c45a
> [2] https://cgit.freebsd.org/src/commit/?h=releng/12.2&id=a80adba5ab46ba6d44d5abfc9b7f3b6de8afda55
> [3] https://sources.debian.org/src/openssh/1%3A9.8p1-1/debian/changelog/#L895

Thanks for looking into this.

Generally, I try to follow a principle that if an interface is
genuinely new functionality, managing some new kind of kernel object
or something previously not in the data model, that it's fine not to
have fallback, but that if it's just a new way to act on existing
things (like adding a missing flags argument to an existing
operation), there should be fallback at least in cases where no new
underlying functionality is needed (like in that example, if flags
value is 0).

However, close_range really isn't an improved/generalized way to do
close, but something intended for its own purposes, and if
applications which use it are prepared for it to fail with ENOSYS
(this is important! not just prepared for it to be missing at
configure-time link check) then omitting fallback and letting them do
their own fallbacks seems like it'd be okay.

Unlike closefrom, whose *only* use is invoking UB by closing fds you
don't own, close_range at least admits well-defined uses where you've
tracked ranges you do own and want to close them quickly. That's not
to say folks will use it this way, but having at least some valid use
is something going for it.

Without a fallback, I'm not sure there's a lot of value to providing a
wrapper rather than just having applications use it via
syscall(SYS_close_range, ...), but there's also not any significant
cost to having the wrapper if that's what programs expect.

Are there any other things we should weigh on this topic?

Rich