Message ID | 20211108172817.2235239-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] io: Refactor close_range and closefrom | expand |
If no one oposes it I will commit this shortly. On 08/11/2021 14:28, Adhemerval Zanella wrote: > Now that Hurd implementis both close_range and closefrom (f2c996597d), > we can make close_range() a base ABI, and make the default closefrom() > implementation on top of close_range(). > > The generic closefrom() implementation based on __getdtablesize() is > moved to generic close_range(). On Linux it will be overriden by > the auto-generation syscall while on Hurd it will be a system specific > implementation. > > The closefrom() now calls close_range() and __closefrom_fallback(). > Since on Hurd close_range() does not fail, __closefrom_fallback() is an > empty static inline function set by__ASSUME_CLOSE_RANGE. > > The __ASSUME_CLOSE_RANGE also allows optimize Linux > __closefrom_fallback() implementation when --enable-kernel=5.9 or > higher is used. > > Finally the Linux specific tst-close_range.c is moved to io and > enabled as default. The Linuxism and CLOSE_RANGE_UNSHARE are > guarded so it can be built for Hurd (I have not actually test it). > > Checked on x86_64-linux-gnu, i686-linux-gnu, and with a i686-gnu > build. > --- > Changes from v1: > - Changed close_range() comment to specify close() errors are > ignored. > - Fixed close_range() default implementation. > - __closefrom_fallback() for __ASSUME_CLOSE_RANGE. > --- > include/unistd.h | 10 ++++++ > io/Makefile | 3 +- > .../linux/closefrom.c => io/close_range.c | 34 ++++++++++++------- > io/closefrom.c | 16 +++++---- > .../unix/sysv/linux => io}/tst-close_range.c | 8 +++++ > posix/unistd.h | 10 ++++++ > sysdeps/mach/hurd/Makefile | 2 +- > sysdeps/mach/hurd/bits/unistd_ext.h | 6 ---- > sysdeps/mach/hurd/closefrom.c | 29 ---------------- > sysdeps/mach/hurd/kernel-features.h | 2 ++ > sysdeps/unix/sysv/linux/Makefile | 1 - > sysdeps/unix/sysv/linux/bits/unistd_ext.h | 9 ----- > sysdeps/unix/sysv/linux/closefrom_fallback.c | 4 +++ > sysdeps/unix/sysv/linux/kernel-features.h | 8 +++++ > sysdeps/unix/sysv/linux/syscalls.list | 2 +- > 15 files changed, 76 insertions(+), 68 deletions(-) > rename sysdeps/unix/sysv/linux/closefrom.c => io/close_range.c (59%) > rename {sysdeps/unix/sysv/linux => io}/tst-close_range.c (98%) > delete mode 100644 sysdeps/mach/hurd/closefrom.c > > diff --git a/include/unistd.h b/include/unistd.h > index 7849562c42..927d249380 100644 > --- a/include/unistd.h > +++ b/include/unistd.h > @@ -3,6 +3,9 @@ > > # ifndef _ISOMAC > > +# include <stdbool.h> > +# include <kernel-features.h> > + > libc_hidden_proto (_exit, __noreturn__) > # ifndef NO_RTLD_HIDDEN > rtld_hidden_proto (_exit, __noreturn__) > @@ -158,7 +161,14 @@ extern int __brk (void *__addr) attribute_hidden; > extern int __close (int __fd); > libc_hidden_proto (__close) > extern int __libc_close (int __fd); > +#if __ASSUME_CLOSE_RANGE > +static inline _Bool __closefrom_fallback (int __lowfd, _Bool dirfd_fallback) > +{ > + return false; > +} > +#else > extern _Bool __closefrom_fallback (int __lowfd, _Bool) attribute_hidden; > +#endif > extern ssize_t __read (int __fd, void *__buf, size_t __nbytes); > libc_hidden_proto (__read) > extern ssize_t __write (int __fd, const void *__buf, size_t __n); > diff --git a/io/Makefile b/io/Makefile > index ecf65aba60..83f6ffdb76 100644 > --- a/io/Makefile > +++ b/io/Makefile > @@ -57,7 +57,7 @@ routines := \ > utimensat futimens file_change_detection \ > fts64-time64 \ > ftw64-time64 \ > - closefrom > + closefrom close_range > > others := pwd > test-srcs := ftwtest ftwtest-time64 > @@ -79,6 +79,7 @@ tests := test-utime test-stat test-stat2 test-lfs tst-getcwd \ > tst-futimens \ > tst-utimensat \ > tst-closefrom \ > + tst-close_range \ > tst-ftw-bz28126 > > tests-time64 := \ > diff --git a/sysdeps/unix/sysv/linux/closefrom.c b/io/close_range.c > similarity index 59% > rename from sysdeps/unix/sysv/linux/closefrom.c > rename to io/close_range.c > index 372896b775..26a615d8b9 100644 > --- a/sysdeps/unix/sysv/linux/closefrom.c > +++ b/io/close_range.c > @@ -1,4 +1,4 @@ > -/* Close a range of file descriptors. Linux version. > +/* Close a range of file descriptors. > Copyright (C) 2021 Free Software Foundation, Inc. > This file is part of the GNU C Library. > > @@ -16,21 +16,29 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > -#include <stdbool.h> > -#include <stdio.h> > -#include <sys/param.h> > +#include <errno.h> > +#include <not-cancel.h> > #include <unistd.h> > > -void > -__closefrom (int lowfd) > +/* Close the file descriptors from FIRST up to LAST, inclusive. */ > +int > +__close_range (unsigned int first, unsigned int last, > + int flags) > { > - int l = MAX (0, lowfd); > + if (first > last || flags != 0) > + { > + __set_errno (EINVAL); > + return -1; > + } > > - int r = __close_range (l, ~0U, 0); > - if (r == 0) > - return; > + int maxfd = __getdtablesize (); > + if (maxfd == -1) > + return -1; > > - if (!__closefrom_fallback (l, true)) > - __fortify_fail ("closefrom failed to close a file descriptor"); > + for (int i = first; i <= last && i < maxfd; i++) > + __close_nocancel_nostatus (i); > + > + return 0; > } > -weak_alias (__closefrom, closefrom) > +libc_hidden_def (__close_range) > +weak_alias (__close_range, close_range) > diff --git a/io/closefrom.c b/io/closefrom.c > index 01660a7531..e9167687bc 100644 > --- a/io/closefrom.c > +++ b/io/closefrom.c > @@ -16,19 +16,21 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#include <stdbool.h> > #include <stdio.h> > +#include <sys/param.h> > #include <unistd.h> > -#include <not-cancel.h> > > void > __closefrom (int lowfd) > { > - int maxfd = __getdtablesize (); > - if (maxfd == -1) > - __fortify_fail ("closefrom failed to get the file descriptor table size"); > + int l = MAX (0, lowfd); > > - for (int i = 0; i < maxfd; i++) > - if (i >= lowfd) > - __close_nocancel_nostatus (i); > + int r = __close_range (l, ~0U, 0); > + if (r == 0) > + return ; > + > + if (!__closefrom_fallback (l, true)) > + __fortify_fail ("closefrom failed to close a file descriptor"); > } > weak_alias (__closefrom, closefrom) > diff --git a/sysdeps/unix/sysv/linux/tst-close_range.c b/io/tst-close_range.c > similarity index 98% > rename from sysdeps/unix/sysv/linux/tst-close_range.c > rename to io/tst-close_range.c > index f5069d1b8a..f05b4ff6ae 100644 > --- a/sysdeps/unix/sysv/linux/tst-close_range.c > +++ b/io/tst-close_range.c > @@ -119,6 +119,7 @@ close_range_test (void) > support_descriptors_free (descrs); > } > > +#ifdef __linux__ > _Noreturn static int > close_range_test_fn (void *arg) > { > @@ -155,8 +156,10 @@ close_range_test_subprocess (void) > support_descriptors_check (descrs); > support_descriptors_free (descrs); > } > +#endif > > > +#ifdef CLOSE_RANGE_UNSHARE > _Noreturn static int > close_range_unshare_test_fn (void *arg) > { > @@ -200,6 +203,7 @@ close_range_unshare_test (void) > support_descriptors_check (descrs1); > support_descriptors_free (descrs1); > } > +#endif > > static bool > is_in_array (int *arr, size_t len, int fd) > @@ -282,8 +286,12 @@ do_test (void) > { > close_range_test_max_upper_limit (); > close_range_test (); > +#ifdef __linux__ > close_range_test_subprocess (); > +#endif > +#ifdef CLOSE_RANGE_UNSHARE > close_range_unshare_test (); > +#endif > close_range_cloexec_test (); > > return 0; > diff --git a/posix/unistd.h b/posix/unistd.h > index 7a61ff5e86..3c8a7ced6a 100644 > --- a/posix/unistd.h > +++ b/posix/unistd.h > @@ -1199,6 +1199,16 @@ int getentropy (void *__buffer, size_t __length) __wur > __attr_access ((__write_only__, 1, 2)); > #endif > > +#ifdef __USE_GNU > +/* Close all file descriptors in the range FD up to MAX_FD. The flag FLAGS > + are define by the CLOSE_RANGE prefix. This function behaves like close > + on the range and gaps where the file descriptor is invalid or errors > + encountered while closing file descriptors are ignored. Returns 0 on > + successor or -1 for failure (and sets errno accordingly). */ > +extern int close_range (unsigned int __fd, unsigned int __max_fd, > + int __flags) __THROW; > +#endif > + > /* Define some macros helping to catch buffer overflows. */ > #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function > # include <bits/unistd.h> > diff --git a/sysdeps/mach/hurd/Makefile b/sysdeps/mach/hurd/Makefile > index 9acbe80f26..17bb643c18 100644 > --- a/sysdeps/mach/hurd/Makefile > +++ b/sysdeps/mach/hurd/Makefile > @@ -196,7 +196,7 @@ sysdep_routines += cthreads > endif > > ifeq (io, $(subdir)) > -sysdep_routines += f_setlk close_nocancel close_nocancel_nostatus close_range \ > +sysdep_routines += f_setlk close_nocancel close_nocancel_nostatus \ > fcntl_nocancel open_nocancel openat_nocancel read_nocancel \ > pread64_nocancel write_nocancel pwrite64_nocancel \ > wait4_nocancel \ > diff --git a/sysdeps/mach/hurd/bits/unistd_ext.h b/sysdeps/mach/hurd/bits/unistd_ext.h > index 288f504a3c..14f85539d5 100644 > --- a/sysdeps/mach/hurd/bits/unistd_ext.h > +++ b/sysdeps/mach/hurd/bits/unistd_ext.h > @@ -25,10 +25,4 @@ > /* Set the FD_CLOEXEC bit instead of closing the file descriptor. */ > #define CLOSE_RANGE_CLOEXEC (1U << 2) > > -/* Close the file descriptors from FIRST up to LAST, inclusive. > - If CLOSE_RANGE_CLOEXEC is set in FLAGS, set the FD_CLOEXEC flag > - instead of closing. */ > -extern int close_range (unsigned int __first, unsigned int __last, > - int __flags) __THROW; > - > #endif /* __USE_GNU */ > diff --git a/sysdeps/mach/hurd/closefrom.c b/sysdeps/mach/hurd/closefrom.c > deleted file mode 100644 > index 5d667cf6c4..0000000000 > --- a/sysdeps/mach/hurd/closefrom.c > +++ /dev/null > @@ -1,29 +0,0 @@ > -/* Close a range of file descriptors. Hurd version. > - Copyright (C) 2021 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - <https://www.gnu.org/licenses/>. */ > - > -#include <unistd.h> > -#include <sys/param.h> > - > -void > -__closefrom (int lowfd) > -{ > - int l = MAX (0, lowfd); > - > - (void) __close_range (l, ~0U, 0); > -} > -weak_alias (__closefrom, closefrom) > diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h > index 7d4eaee0a6..5fd37a6d7b 100644 > --- a/sysdeps/mach/hurd/kernel-features.h > +++ b/sysdeps/mach/hurd/kernel-features.h > @@ -19,3 +19,5 @@ > /* This file can define __ASSUME_* macros checked by certain source files. > Almost none of these are used outside of sysdeps/unix/sysv/linux code. > But those referring to POSIX-level features like O_* flags can be. */ > + > +#define __ASSUME_CLOSE_RANGE 1 > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 76ad06361c..76042a6019 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -120,7 +120,6 @@ tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ > tst-timerfd tst-ppoll \ > tst-clock_adjtime tst-adjtimex tst-ntp_adjtime tst-ntp_gettime \ > tst-ntp_gettimex tst-sigtimedwait tst-misalign-clone \ > - tst-close_range \ > tst-prctl \ > tst-scm_rights \ > # tests > diff --git a/sysdeps/unix/sysv/linux/bits/unistd_ext.h b/sysdeps/unix/sysv/linux/bits/unistd_ext.h > index ae9994403c..8f422e60da 100644 > --- a/sysdeps/unix/sysv/linux/bits/unistd_ext.h > +++ b/sysdeps/unix/sysv/linux/bits/unistd_ext.h > @@ -47,13 +47,4 @@ extern __pid_t gettid (void) __THROW; > # define CLOSE_RANGE_CLOEXEC (1U << 2) > #endif > > -/* Close all file descriptors in the range FD up to MAX_FD. The flag FLAGS > - are define by the CLOSE_RANGE prefix. This function behaves like close > - on the range, but in a fail-safe where it will either fail and not close > - any file descriptor or close all of them. Gaps where the file descriptor > - is invalid are ignored. Returns 0 on successor or -1 for failure (and > - sets errno accordingly). */ > -extern int close_range (unsigned int __fd, unsigned int __max_fd, > - int __flags) __THROW; > - > #endif /* __USE_GNU */ > diff --git a/sysdeps/unix/sysv/linux/closefrom_fallback.c b/sysdeps/unix/sysv/linux/closefrom_fallback.c > index f215fd2c09..a377ebc544 100644 > --- a/sysdeps/unix/sysv/linux/closefrom_fallback.c > +++ b/sysdeps/unix/sysv/linux/closefrom_fallback.c > @@ -21,6 +21,8 @@ > #include <not-cancel.h> > #include <stdbool.h> > > +#if !__ASSUME_CLOSE_RANGE > + > /* Fallback code: iterates over /proc/self/fd, closing each file descriptor > that fall on the criteria. If DIRFD_FALLBACK is set, a failure on > /proc/self/fd open will trigger a fallback that tries to close a file > @@ -97,3 +99,5 @@ err: > __close_nocancel (dirfd); > return ret; > } > + > +#endif > diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h > index ffb6af196b..a1108d434f 100644 > --- a/sysdeps/unix/sysv/linux/kernel-features.h > +++ b/sysdeps/unix/sysv/linux/kernel-features.h > @@ -220,6 +220,14 @@ > # define __ASSUME_FACCESSAT2 0 > #endif > > +/* The close_range system call was introduced across all architectures > + in Linux 5.8. */ > +#if __LINUX_KERNEL_VERSION >= 0x050900 > +# define __ASSUME_CLOSE_RANGE 1 > +#else > +# define __ASSUME_CLOSE_RANGE 0 > +#endif > + > /* The FUTEX_LOCK_PI2 operation was introduced across all architectures in Linux > 5.14. */ > #if __LINUX_KERNEL_VERSION >= 0x050e00 > diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list > index 29899eb264..c38dbb34a1 100644 > --- a/sysdeps/unix/sysv/linux/syscalls.list > +++ b/sysdeps/unix/sysv/linux/syscalls.list > @@ -99,4 +99,4 @@ pkey_alloc EXTRA pkey_alloc i:ii pkey_alloc > pkey_free EXTRA pkey_free i:i pkey_free > gettid EXTRA gettid Ei: __gettid gettid > tgkill EXTRA tgkill i:iii __tgkill tgkill > -close_range EXTRA close_range i:iii __close_range close_range > +close_range - close_range i:iii __close_range close_range >
* Adhemerval Zanella via Libc-alpha: > diff --git a/include/unistd.h b/include/unistd.h > index 7849562c42..927d249380 100644 > --- a/include/unistd.h > +++ b/include/unistd.h > @@ -3,6 +3,9 @@ > > # ifndef _ISOMAC > > +# include <stdbool.h> > +# include <kernel-features.h> > + > libc_hidden_proto (_exit, __noreturn__) > # ifndef NO_RTLD_HIDDEN > rtld_hidden_proto (_exit, __noreturn__) > @@ -158,7 +161,14 @@ extern int __brk (void *__addr) attribute_hidden; > extern int __close (int __fd); > libc_hidden_proto (__close) > extern int __libc_close (int __fd); > +#if __ASSUME_CLOSE_RANGE > +static inline _Bool __closefrom_fallback (int __lowfd, _Bool dirfd_fallback) > +{ > + return false; > +} > +#else > extern _Bool __closefrom_fallback (int __lowfd, _Bool) attribute_hidden; > +#endif > extern ssize_t __read (int __fd, void *__buf, size_t __nbytes); > libc_hidden_proto (__read) > extern ssize_t __write (int __fd, const void *__buf, size_t __n); #if indentation seems off. I expect this will break the Hurd build because it does not define __ASSUME_CLOSE_RANGE in its <kernel-features.h>. Thanks, Florian
On 24/11/2021 06:52, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> diff --git a/include/unistd.h b/include/unistd.h >> index 7849562c42..927d249380 100644 >> --- a/include/unistd.h >> +++ b/include/unistd.h >> @@ -3,6 +3,9 @@ >> >> # ifndef _ISOMAC >> >> +# include <stdbool.h> >> +# include <kernel-features.h> >> + >> libc_hidden_proto (_exit, __noreturn__) >> # ifndef NO_RTLD_HIDDEN >> rtld_hidden_proto (_exit, __noreturn__) >> @@ -158,7 +161,14 @@ extern int __brk (void *__addr) attribute_hidden; >> extern int __close (int __fd); >> libc_hidden_proto (__close) >> extern int __libc_close (int __fd); >> +#if __ASSUME_CLOSE_RANGE >> +static inline _Bool __closefrom_fallback (int __lowfd, _Bool dirfd_fallback) >> +{ >> + return false; >> +} >> +#else >> extern _Bool __closefrom_fallback (int __lowfd, _Bool) attribute_hidden; >> +#endif >> extern ssize_t __read (int __fd, void *__buf, size_t __nbytes); >> libc_hidden_proto (__read) >> extern ssize_t __write (int __fd, const void *__buf, size_t __n); > > #if indentation seems off. Ack. > > I expect this will break the Hurd build because it does not define > __ASSUME_CLOSE_RANGE in its <kernel-features.h>. > diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h > index 7d4eaee0a6..5fd37a6d7b 100644 > --- a/sysdeps/mach/hurd/kernel-features.h > +++ b/sysdeps/mach/hurd/kernel-features.h > @@ -19,3 +19,5 @@ > /* This file can define __ASSUME_* macros checked by certain source files. > Almost none of these are used outside of sysdeps/unix/sysv/linux code. > But those referring to POSIX-level features like O_* flags can be. */ > + > +#define __ASSUME_CLOSE_RANGE 1 It does add it here (I also checked with a build for i688-gnu). > > Thanks, > Florian >
* Adhemerval Zanella: >> diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h >> index 7d4eaee0a6..5fd37a6d7b 100644 >> --- a/sysdeps/mach/hurd/kernel-features.h >> +++ b/sysdeps/mach/hurd/kernel-features.h >> @@ -19,3 +19,5 @@ >> /* This file can define __ASSUME_* macros checked by certain source files. >> Almost none of these are used outside of sysdeps/unix/sysv/linux code. >> But those referring to POSIX-level features like O_* flags can be. */ >> + >> +#define __ASSUME_CLOSE_RANGE 1 > > It does add it here (I also checked with a build for i688-gnu). Hmm, this doesn't match existing practice. We haven't unified implementations in light of such divergence, I think. If the Hurd developers don't object, it should be fine, though.. Thanks, Florian
Florian Weimer, le mer. 24 nov. 2021 13:03:19 +0100, a ecrit: > * Adhemerval Zanella: > > >> diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h > >> index 7d4eaee0a6..5fd37a6d7b 100644 > >> --- a/sysdeps/mach/hurd/kernel-features.h > >> +++ b/sysdeps/mach/hurd/kernel-features.h > >> @@ -19,3 +19,5 @@ > >> /* This file can define __ASSUME_* macros checked by certain source files. > >> Almost none of these are used outside of sysdeps/unix/sysv/linux code. > >> But those referring to POSIX-level features like O_* flags can be. */ > >> + > >> +#define __ASSUME_CLOSE_RANGE 1 > > > > It does add it here (I also checked with a build for i688-gnu). > > Hmm, this doesn't match existing practice. We haven't unified > implementations in light of such divergence, I think. If the Hurd > developers don't object, it should be fine, though.. I'm fine with it yes. This is not a "kernel-feature" since it's actually implemented inside glibc itself, but from the point of view of the rest of the world that's how people understand it, so better adhere to it :) Samuel
On 24/11/2021 09:03, Florian Weimer wrote: > * Adhemerval Zanella: > >>> diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h >>> index 7d4eaee0a6..5fd37a6d7b 100644 >>> --- a/sysdeps/mach/hurd/kernel-features.h >>> +++ b/sysdeps/mach/hurd/kernel-features.h >>> @@ -19,3 +19,5 @@ >>> /* This file can define __ASSUME_* macros checked by certain source files. >>> Almost none of these are used outside of sysdeps/unix/sysv/linux code. >>> But those referring to POSIX-level features like O_* flags can be. */ >>> + >>> +#define __ASSUME_CLOSE_RANGE 1 >> >> It does add it here (I also checked with a build for i688-gnu). > > Hmm, this doesn't match existing practice. We haven't unified > implementations in light of such divergence, I think. If the Hurd > developers don't object, it should be fine, though.. This was used before, we still have some leftovers from it: nscd/nscd.c:306:# ifndef __ASSUME_IN_NONBLOCK resolv/res_send.c:1035:#ifndef __ASSUME_SENDMMSG resolv/res_send.c:1081:#ifndef __ASSUME_SENDMMSG resolv/res_send.c:1100:#ifndef __ASSUME_SENDMMSG You have also cleanup some other assumes from Hurd: dup3 (b48061e1a534a242), O_CLOEXEC (cef9b65376a04430), and accept4 (e92030239abb4038).
On 24/11/2021 09:20, Adhemerval Zanella wrote: > > > On 24/11/2021 09:03, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>>> diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h >>>> index 7d4eaee0a6..5fd37a6d7b 100644 >>>> --- a/sysdeps/mach/hurd/kernel-features.h >>>> +++ b/sysdeps/mach/hurd/kernel-features.h >>>> @@ -19,3 +19,5 @@ >>>> /* This file can define __ASSUME_* macros checked by certain source files. >>>> Almost none of these are used outside of sysdeps/unix/sysv/linux code. >>>> But those referring to POSIX-level features like O_* flags can be. */ >>>> + >>>> +#define __ASSUME_CLOSE_RANGE 1 >>> >>> It does add it here (I also checked with a build for i688-gnu). >> >> Hmm, this doesn't match existing practice. We haven't unified >> implementations in light of such divergence, I think. If the Hurd >> developers don't object, it should be fine, though.. > > This was used before, we still have some leftovers from it: > > nscd/nscd.c:306:# ifndef __ASSUME_IN_NONBLOCK On a side note, I think we can just remove it since it is only used with inotify_init1 (which is only supported on Linux and currently on all minimum supported kernel). > resolv/res_send.c:1035:#ifndef __ASSUME_SENDMMSG > resolv/res_send.c:1081:#ifndef __ASSUME_SENDMMSG > resolv/res_send.c:1100:#ifndef __ASSUME_SENDMMSG And we might remove it by implementing sendmmsg/recvmmsg on top of sendmsg/recvmsg on generic implementations. > > You have also cleanup some other assumes from Hurd: dup3 (b48061e1a534a242), > O_CLOEXEC (cef9b65376a04430), and accept4 (e92030239abb4038). >
diff --git a/include/unistd.h b/include/unistd.h index 7849562c42..927d249380 100644 --- a/include/unistd.h +++ b/include/unistd.h @@ -3,6 +3,9 @@ # ifndef _ISOMAC +# include <stdbool.h> +# include <kernel-features.h> + libc_hidden_proto (_exit, __noreturn__) # ifndef NO_RTLD_HIDDEN rtld_hidden_proto (_exit, __noreturn__) @@ -158,7 +161,14 @@ extern int __brk (void *__addr) attribute_hidden; extern int __close (int __fd); libc_hidden_proto (__close) extern int __libc_close (int __fd); +#if __ASSUME_CLOSE_RANGE +static inline _Bool __closefrom_fallback (int __lowfd, _Bool dirfd_fallback) +{ + return false; +} +#else extern _Bool __closefrom_fallback (int __lowfd, _Bool) attribute_hidden; +#endif extern ssize_t __read (int __fd, void *__buf, size_t __nbytes); libc_hidden_proto (__read) extern ssize_t __write (int __fd, const void *__buf, size_t __n); diff --git a/io/Makefile b/io/Makefile index ecf65aba60..83f6ffdb76 100644 --- a/io/Makefile +++ b/io/Makefile @@ -57,7 +57,7 @@ routines := \ utimensat futimens file_change_detection \ fts64-time64 \ ftw64-time64 \ - closefrom + closefrom close_range others := pwd test-srcs := ftwtest ftwtest-time64 @@ -79,6 +79,7 @@ tests := test-utime test-stat test-stat2 test-lfs tst-getcwd \ tst-futimens \ tst-utimensat \ tst-closefrom \ + tst-close_range \ tst-ftw-bz28126 tests-time64 := \ diff --git a/sysdeps/unix/sysv/linux/closefrom.c b/io/close_range.c similarity index 59% rename from sysdeps/unix/sysv/linux/closefrom.c rename to io/close_range.c index 372896b775..26a615d8b9 100644 --- a/sysdeps/unix/sysv/linux/closefrom.c +++ b/io/close_range.c @@ -1,4 +1,4 @@ -/* Close a range of file descriptors. Linux version. +/* Close a range of file descriptors. Copyright (C) 2021 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -16,21 +16,29 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#include <stdbool.h> -#include <stdio.h> -#include <sys/param.h> +#include <errno.h> +#include <not-cancel.h> #include <unistd.h> -void -__closefrom (int lowfd) +/* Close the file descriptors from FIRST up to LAST, inclusive. */ +int +__close_range (unsigned int first, unsigned int last, + int flags) { - int l = MAX (0, lowfd); + if (first > last || flags != 0) + { + __set_errno (EINVAL); + return -1; + } - int r = __close_range (l, ~0U, 0); - if (r == 0) - return; + int maxfd = __getdtablesize (); + if (maxfd == -1) + return -1; - if (!__closefrom_fallback (l, true)) - __fortify_fail ("closefrom failed to close a file descriptor"); + for (int i = first; i <= last && i < maxfd; i++) + __close_nocancel_nostatus (i); + + return 0; } -weak_alias (__closefrom, closefrom) +libc_hidden_def (__close_range) +weak_alias (__close_range, close_range) diff --git a/io/closefrom.c b/io/closefrom.c index 01660a7531..e9167687bc 100644 --- a/io/closefrom.c +++ b/io/closefrom.c @@ -16,19 +16,21 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#include <stdbool.h> #include <stdio.h> +#include <sys/param.h> #include <unistd.h> -#include <not-cancel.h> void __closefrom (int lowfd) { - int maxfd = __getdtablesize (); - if (maxfd == -1) - __fortify_fail ("closefrom failed to get the file descriptor table size"); + int l = MAX (0, lowfd); - for (int i = 0; i < maxfd; i++) - if (i >= lowfd) - __close_nocancel_nostatus (i); + int r = __close_range (l, ~0U, 0); + if (r == 0) + return ; + + if (!__closefrom_fallback (l, true)) + __fortify_fail ("closefrom failed to close a file descriptor"); } weak_alias (__closefrom, closefrom) diff --git a/sysdeps/unix/sysv/linux/tst-close_range.c b/io/tst-close_range.c similarity index 98% rename from sysdeps/unix/sysv/linux/tst-close_range.c rename to io/tst-close_range.c index f5069d1b8a..f05b4ff6ae 100644 --- a/sysdeps/unix/sysv/linux/tst-close_range.c +++ b/io/tst-close_range.c @@ -119,6 +119,7 @@ close_range_test (void) support_descriptors_free (descrs); } +#ifdef __linux__ _Noreturn static int close_range_test_fn (void *arg) { @@ -155,8 +156,10 @@ close_range_test_subprocess (void) support_descriptors_check (descrs); support_descriptors_free (descrs); } +#endif +#ifdef CLOSE_RANGE_UNSHARE _Noreturn static int close_range_unshare_test_fn (void *arg) { @@ -200,6 +203,7 @@ close_range_unshare_test (void) support_descriptors_check (descrs1); support_descriptors_free (descrs1); } +#endif static bool is_in_array (int *arr, size_t len, int fd) @@ -282,8 +286,12 @@ do_test (void) { close_range_test_max_upper_limit (); close_range_test (); +#ifdef __linux__ close_range_test_subprocess (); +#endif +#ifdef CLOSE_RANGE_UNSHARE close_range_unshare_test (); +#endif close_range_cloexec_test (); return 0; diff --git a/posix/unistd.h b/posix/unistd.h index 7a61ff5e86..3c8a7ced6a 100644 --- a/posix/unistd.h +++ b/posix/unistd.h @@ -1199,6 +1199,16 @@ int getentropy (void *__buffer, size_t __length) __wur __attr_access ((__write_only__, 1, 2)); #endif +#ifdef __USE_GNU +/* Close all file descriptors in the range FD up to MAX_FD. The flag FLAGS + are define by the CLOSE_RANGE prefix. This function behaves like close + on the range and gaps where the file descriptor is invalid or errors + encountered while closing file descriptors are ignored. Returns 0 on + successor or -1 for failure (and sets errno accordingly). */ +extern int close_range (unsigned int __fd, unsigned int __max_fd, + int __flags) __THROW; +#endif + /* Define some macros helping to catch buffer overflows. */ #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function # include <bits/unistd.h> diff --git a/sysdeps/mach/hurd/Makefile b/sysdeps/mach/hurd/Makefile index 9acbe80f26..17bb643c18 100644 --- a/sysdeps/mach/hurd/Makefile +++ b/sysdeps/mach/hurd/Makefile @@ -196,7 +196,7 @@ sysdep_routines += cthreads endif ifeq (io, $(subdir)) -sysdep_routines += f_setlk close_nocancel close_nocancel_nostatus close_range \ +sysdep_routines += f_setlk close_nocancel close_nocancel_nostatus \ fcntl_nocancel open_nocancel openat_nocancel read_nocancel \ pread64_nocancel write_nocancel pwrite64_nocancel \ wait4_nocancel \ diff --git a/sysdeps/mach/hurd/bits/unistd_ext.h b/sysdeps/mach/hurd/bits/unistd_ext.h index 288f504a3c..14f85539d5 100644 --- a/sysdeps/mach/hurd/bits/unistd_ext.h +++ b/sysdeps/mach/hurd/bits/unistd_ext.h @@ -25,10 +25,4 @@ /* Set the FD_CLOEXEC bit instead of closing the file descriptor. */ #define CLOSE_RANGE_CLOEXEC (1U << 2) -/* Close the file descriptors from FIRST up to LAST, inclusive. - If CLOSE_RANGE_CLOEXEC is set in FLAGS, set the FD_CLOEXEC flag - instead of closing. */ -extern int close_range (unsigned int __first, unsigned int __last, - int __flags) __THROW; - #endif /* __USE_GNU */ diff --git a/sysdeps/mach/hurd/closefrom.c b/sysdeps/mach/hurd/closefrom.c deleted file mode 100644 index 5d667cf6c4..0000000000 --- a/sysdeps/mach/hurd/closefrom.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Close a range of file descriptors. Hurd version. - Copyright (C) 2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#include <unistd.h> -#include <sys/param.h> - -void -__closefrom (int lowfd) -{ - int l = MAX (0, lowfd); - - (void) __close_range (l, ~0U, 0); -} -weak_alias (__closefrom, closefrom) diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h index 7d4eaee0a6..5fd37a6d7b 100644 --- a/sysdeps/mach/hurd/kernel-features.h +++ b/sysdeps/mach/hurd/kernel-features.h @@ -19,3 +19,5 @@ /* This file can define __ASSUME_* macros checked by certain source files. Almost none of these are used outside of sysdeps/unix/sysv/linux code. But those referring to POSIX-level features like O_* flags can be. */ + +#define __ASSUME_CLOSE_RANGE 1 diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 76ad06361c..76042a6019 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -120,7 +120,6 @@ tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ tst-timerfd tst-ppoll \ tst-clock_adjtime tst-adjtimex tst-ntp_adjtime tst-ntp_gettime \ tst-ntp_gettimex tst-sigtimedwait tst-misalign-clone \ - tst-close_range \ tst-prctl \ tst-scm_rights \ # tests diff --git a/sysdeps/unix/sysv/linux/bits/unistd_ext.h b/sysdeps/unix/sysv/linux/bits/unistd_ext.h index ae9994403c..8f422e60da 100644 --- a/sysdeps/unix/sysv/linux/bits/unistd_ext.h +++ b/sysdeps/unix/sysv/linux/bits/unistd_ext.h @@ -47,13 +47,4 @@ extern __pid_t gettid (void) __THROW; # define CLOSE_RANGE_CLOEXEC (1U << 2) #endif -/* Close all file descriptors in the range FD up to MAX_FD. The flag FLAGS - are define by the CLOSE_RANGE prefix. This function behaves like close - on the range, but in a fail-safe where it will either fail and not close - any file descriptor or close all of them. Gaps where the file descriptor - is invalid are ignored. Returns 0 on successor or -1 for failure (and - sets errno accordingly). */ -extern int close_range (unsigned int __fd, unsigned int __max_fd, - int __flags) __THROW; - #endif /* __USE_GNU */ diff --git a/sysdeps/unix/sysv/linux/closefrom_fallback.c b/sysdeps/unix/sysv/linux/closefrom_fallback.c index f215fd2c09..a377ebc544 100644 --- a/sysdeps/unix/sysv/linux/closefrom_fallback.c +++ b/sysdeps/unix/sysv/linux/closefrom_fallback.c @@ -21,6 +21,8 @@ #include <not-cancel.h> #include <stdbool.h> +#if !__ASSUME_CLOSE_RANGE + /* Fallback code: iterates over /proc/self/fd, closing each file descriptor that fall on the criteria. If DIRFD_FALLBACK is set, a failure on /proc/self/fd open will trigger a fallback that tries to close a file @@ -97,3 +99,5 @@ err: __close_nocancel (dirfd); return ret; } + +#endif diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h index ffb6af196b..a1108d434f 100644 --- a/sysdeps/unix/sysv/linux/kernel-features.h +++ b/sysdeps/unix/sysv/linux/kernel-features.h @@ -220,6 +220,14 @@ # define __ASSUME_FACCESSAT2 0 #endif +/* The close_range system call was introduced across all architectures + in Linux 5.8. */ +#if __LINUX_KERNEL_VERSION >= 0x050900 +# define __ASSUME_CLOSE_RANGE 1 +#else +# define __ASSUME_CLOSE_RANGE 0 +#endif + /* The FUTEX_LOCK_PI2 operation was introduced across all architectures in Linux 5.14. */ #if __LINUX_KERNEL_VERSION >= 0x050e00 diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index 29899eb264..c38dbb34a1 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -99,4 +99,4 @@ pkey_alloc EXTRA pkey_alloc i:ii pkey_alloc pkey_free EXTRA pkey_free i:i pkey_free gettid EXTRA gettid Ei: __gettid gettid tgkill EXTRA tgkill i:iii __tgkill tgkill -close_range EXTRA close_range i:iii __close_range close_range +close_range - close_range i:iii __close_range close_range