I found some more problems, updated patch attached. On Wed, 25 Aug 2021 at 15:41, Michael Hudson-Doyle < michael.hudson@canonical.com> wrote: > > > On Wed, 25 Aug 2021 at 15:15, Michael Hudson-Doyle < > michael.hudson@canonical.com> wrote: > >> Hi, thanks for looking at this. >> >> On Wed, 25 Aug 2021 at 07:29, Adhemerval Zanella via Libc-alpha < >> libc-alpha@sourceware.org> wrote: >> >>> It ensures a continuous range of file descriptor and avoid hitting >>> the RLIMIT_NOFILE. >>> >>> Checked on x86_64-linux-gnu. >>> --- >>> io/tst-closefrom.c | 15 +++----------- >>> posix/tst-spawn5.c | 13 +----------- >>> sysdeps/unix/sysv/linux/tst-close_range.c | 25 +++++++---------------- >>> 3 files changed, 11 insertions(+), 42 deletions(-) >>> >>> diff --git a/io/tst-closefrom.c b/io/tst-closefrom.c >>> index d4c187073c..0800e19f3f 100644 >>> --- a/io/tst-closefrom.c >>> +++ b/io/tst-closefrom.c >>> @@ -24,29 +24,20 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include >>> >>> #define NFDS 100 >>> >>> -static int >>> -open_multiple_temp_files (void) >>> -{ >>> - /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = xopen ("/dev/null", O_RDONLY, 0600); >>> - for (int i = 1; i <= NFDS; i++) >>> - TEST_COMPARE (xopen ("/dev/null", O_RDONLY, 0600), lowfd + i); >>> - return lowfd; >>> -} >>> - >>> static int >>> closefrom_test (void) >>> { >>> struct support_descriptors *descrs = support_descriptors_list (); >>> >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> - const int maximum_fd = lowfd + NFDS; >>> + const int maximum_fd = lowfd + NFDS - 1; >>> const int half_fd = lowfd + NFDS / 2; >>> const int gap = maximum_fd / 4; >>> >> >> There are two for loops in this function that iterate from 0: >> >> for (int i = 0; i < half_fd; i++) >> TEST_VERIFY (fcntl (i, F_GETFL) > -1); >> >> for (int i = 0; i < gap; i++) >> TEST_VERIFY (fcntl (i, F_GETFL) > -1); >> >> These should both iterate from i = lowfd I think? (And maybe should have >> done before this change?) Certainly they fail in the cases this patch is >> trying to fix. >> >> diff --git a/posix/tst-spawn5.c b/posix/tst-spawn5.c >>> index ac66738004..a95199af6b 100644 >>> --- a/posix/tst-spawn5.c >>> +++ b/posix/tst-spawn5.c >>> @@ -47,17 +47,6 @@ static int initial_argv_count; >>> >>> #define NFDS 100 >>> >>> -static int >>> -open_multiple_temp_files (void) >>> -{ >>> - /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = xopen ("/dev/null", O_RDONLY, 0600); >>> - for (int i = 1; i <= NFDS; i++) >>> - TEST_COMPARE (xopen ("/dev/null", O_RDONLY, 0600), >>> - lowfd + i); >>> - return lowfd; >>> -} >>> - >>> static int >>> parse_fd (const char *str) >>> { >>> @@ -185,7 +174,7 @@ spawn_closefrom_test (posix_spawn_file_actions_t >>> *fa, int lowfd, int highfd, >>> static void >>> do_test_closefrom (void) >>> { >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> const int half_fd = lowfd + NFDS / 2; >>> >>> /* Close half of the descriptors and check result. */ >>> diff --git a/sysdeps/unix/sysv/linux/tst-close_range.c >>> b/sysdeps/unix/sysv/linux/tst-close_range.c >>> index dccb6189c5..3548b10363 100644 >>> --- a/sysdeps/unix/sysv/linux/tst-close_range.c >>> +++ b/sysdeps/unix/sysv/linux/tst-close_range.c >>> >> >> This for look in close_range_unshare_test in this file needs to change >> too: >> >> for (int i = 0; i < NFDS; i++) >> TEST_VERIFY (fcntl (i, F_GETFL) > -1); >> >> (presumably to iterate from low_fd to lowfd + NFDS). >> >> >>> @@ -36,23 +36,12 @@ >>> >>> #define NFDS 100 >>> >>> -static int >>> -open_multiple_temp_files (void) >>> -{ >>> - /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = xopen ("/dev/null", O_RDONLY, 0600); >>> - for (int i = 1; i <= NFDS; i++) >>> - TEST_COMPARE (xopen ("/dev/null", O_RDONLY, 0600), >>> - lowfd + i); >>> - return lowfd; >>> -} >>> - >>> static void >>> close_range_test_max_upper_limit (void) >>> { >>> struct support_descriptors *descrs = support_descriptors_list (); >>> >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> { >>> int r = close_range (lowfd, ~0U, 0); >>> @@ -68,7 +57,7 @@ close_range_test_max_upper_limit (void) >>> static void >>> close_range_test_common (int lowfd, unsigned int flags) >>> { >>> - const int maximum_fd = lowfd + NFDS; >>> + const int maximum_fd = lowfd + NFDS - 1; >>> const int half_fd = lowfd + NFDS / 2; >>> const int gap_1 = maximum_fd - 8; >>> >>> @@ -121,7 +110,7 @@ close_range_test (void) >>> struct support_descriptors *descrs = support_descriptors_list (); >>> >>> /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> close_range_test_common (lowfd, 0); >>> >>> @@ -146,7 +135,7 @@ close_range_test_subprocess (void) >>> struct support_descriptors *descrs = support_descriptors_list (); >>> >>> /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> struct support_stack stack = support_stack_alloc (4096); >>> >>> @@ -184,7 +173,7 @@ close_range_unshare_test (void) >>> struct support_descriptors *descrs1 = support_descriptors_list (); >>> >>> /* Check if the temporary file descriptor has no no gaps. */ >>> - int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> struct support_descriptors *descrs2 = support_descriptors_list (); >>> >>> @@ -226,9 +215,9 @@ static void >>> close_range_cloexec_test (void) >>> { >>> /* Check if the temporary file descriptor has no no gaps. */ >>> - const int lowfd = open_multiple_temp_files (); >>> + int lowfd = support_open_dev_null_range (NFDS, O_RDONLY, 0600); >>> >>> - const int maximum_fd = lowfd + NFDS; >>> + const int maximum_fd = lowfd + NFDS - 1; >>> const int half_fd = lowfd + NFDS / 2; >>> const int gap_1 = maximum_fd - 8; >>> >>> -- >>> 2.30.2 >>> >> >> Cheers, >> mwh >> > > I'm attaching a patch which when applied on top of this one makes these > tests pass my hackish way of reproducing the issue: > > mwhudson@anduril:~/src/pkg/build-glibc$ bash -c 'exec 40 LD_LIBRARY_PATH=. ./elf/ld-linux-x86-64.so.2 ./misc/tst-close_range' > mwhudson@anduril:~/src/pkg/build-glibc$ bash -c 'exec 40 LD_LIBRARY_PATH=. ./elf/ld-linux-x86-64.so.2 ./io/tst-closefrom' > > Cheers, > mwh >