[musl] [PATCH] add close_range() syscall wrapper

[musl] [PATCH] add close_range() syscall wrapper

[Natanael Copa]

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.
---

This is a re-take of close_range() syscall wrapper. It was previously
discussed in mailing list: https://www.openwall.com/lists/musl/2022/08/18/5

Difference from previous submission:

- Use correct values for CLOSE_RANGE_UNSHARE and CLOSE_RANGE_CLOEXEC.
- Set errno on errors.
- Drop the (unsigned int) cast for flags, as it raised questions last
  time.

I think it is a good idea to add this to musl because it is difficult to
the close before exec properly without it. 

Most workaounrds currently out there are either parsing /proc or try to
close everything to maxfd reported by getrlimit(), sysconf() or
getdtablesize().

opendir("/proc/self/fd") is problematic becase 1) /proc may not be
mounted and 2) some versions of musl hangs on malloc between fork and 
exec.

Trying to close everything between a maxfd is also problematic. On some
systems (under docker for example) the maxfd can be 1G, which
effectively results in a hang. (See https://github.com/k0sproject/k0s/pull/3436)

I think its better to encourage the use of close_range().

See also: https://github.com/OpenRC/openrc/pull/645

 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..258ba8bd
--- /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_ret(syscall(SYS_close_range, first, last, flags));
+}
-- 
2.42.0

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

[Rich Felker]

On Fri, Sep 01, 2023 at 10:02:00AM +0200, 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.
> ---
> 
> This is a re-take of close_range() syscall wrapper. It was previously
> discussed in mailing list: https://www.openwall.com/lists/musl/2022/08/18/5
> 
> Difference from previous submission:
> 
> - Use correct values for CLOSE_RANGE_UNSHARE and CLOSE_RANGE_CLOEXEC.
> - Set errno on errors.
> - Drop the (unsigned int) cast for flags, as it raised questions last
>   time.
> 
> I think it is a good idea to add this to musl because it is difficult to
> the close before exec properly without it. 
> 
> Most workaounrds currently out there are either parsing /proc or try to
> close everything to maxfd reported by getrlimit(), sysconf() or
> getdtablesize().
> 
> opendir("/proc/self/fd") is problematic becase 1) /proc may not be
> mounted and 2) some versions of musl hangs on malloc between fork and 
> exec.
> 
> Trying to close everything between a maxfd is also problematic. On some
> systems (under docker for example) the maxfd can be 1G, which
> effectively results in a hang. (See https://github.com/k0sproject/k0s/pull/3436)
> 
> I think its better to encourage the use of close_range().
> 
> See also: https://github.com/OpenRC/openrc/pull/645
> 
>  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..258ba8bd
> --- /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_ret(syscall(SYS_close_range, first, last, flags));
> +}
> -- 
> 2.42.0

This is double-processing errno. You need either return
__syscall_ret(__syscall(...)) (note the second __) or just return
syscall(...) (the syscall macro without __ automatically does the
__syscall_ret).

Aside from that, I think there's a question whether, if we support
this as a function rather than leaving it to the application to use
the syscall, we should provide a fallback for ENOSYS. I'm not sure,
but it's something that should be considered before adding it.

Rich

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

[Natanael Copa]

On Fri, 1 Sep 2023 09:57:34 -0400
Rich Felker <dalias@libc.org> wrote:


> > +int close_range(unsigned int first, unsigned int last, int flags)
> > +{
> > +	return __syscall_ret(syscall(SYS_close_range, first, last, flags));
> > +}
> > -- 
> > 2.42.0  
> 
> This is double-processing errno. You need either return
> __syscall_ret(__syscall(...)) (note the second __) or just return
> syscall(...) (the syscall macro without __ automatically does the
> __syscall_ret).

Ah, ok, I'll send a v2 patch.

> Aside from that, I think there's a question whether, if we support
> this as a function rather than leaving it to the application to use
> the syscall, we should provide a fallback for ENOSYS. I'm not sure,
> but it's something that should be considered before adding it.

It was mentioned earlier that CPython expects close_range() to
async-safe, and that glibc does not provide fallback. I would prefer
that musl does not provide fallback.

https://www.openwall.com/lists/musl/2022/08/18/4

-nc

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

[Natanael Copa]

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);
+}
-- 
2.42.0

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

[Rich Felker]

On Fri, Sep 01, 2023 at 04:55:53PM +0200, Natanael Copa wrote:
> On Fri, 1 Sep 2023 09:57:34 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
> 
> > > +int close_range(unsigned int first, unsigned int last, int flags)
> > > +{
> > > +	return __syscall_ret(syscall(SYS_close_range, first, last, flags));
> > > +}
> > > -- 
> > > 2.42.0  
> > 
> > This is double-processing errno. You need either return
> > __syscall_ret(__syscall(...)) (note the second __) or just return
> > syscall(...) (the syscall macro without __ automatically does the
> > __syscall_ret).
> 
> Ah, ok, I'll send a v2 patch.
> 
> > Aside from that, I think there's a question whether, if we support
> > this as a function rather than leaving it to the application to use
> > the syscall, we should provide a fallback for ENOSYS. I'm not sure,
> > but it's something that should be considered before adding it.
> 
> It was mentioned earlier that CPython expects close_range() to
> async-safe, and that glibc does not provide fallback. I would prefer
> that musl does not provide fallback.
> 
> https://www.openwall.com/lists/musl/2022/08/18/4

If musl were to provide a fallback it would be AS-safe.

Rich