From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 7C4FE3870867 for ; Tue, 9 Mar 2021 09:47:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7C4FE3870867 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-398-D62mbghlM5OZ-4vL0RsPJg-1; Tue, 09 Mar 2021 04:47:56 -0500 X-MC-Unique: D62mbghlM5OZ-4vL0RsPJg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2C67A1009E4C; Tue, 9 Mar 2021 09:47:55 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-112-77.ams2.redhat.com [10.36.112.77]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 207C45D9CD; Tue, 9 Mar 2021 09:47:53 +0000 (UTC) From: Florian Weimer To: Adhemerval Zanella via Libc-alpha Subject: Re: [PATCH v3 2/4] Linux: Add close_range References: <20201223163651.2634504-1-adhemerval.zanella@linaro.org> <20201223163651.2634504-2-adhemerval.zanella@linaro.org> Date: Tue, 09 Mar 2021 10:47:59 +0100 In-Reply-To: <20201223163651.2634504-2-adhemerval.zanella@linaro.org> (Adhemerval Zanella via Libc-alpha's message of "Wed, 23 Dec 2020 13:36:49 -0300") Message-ID: <87mtvc64kw.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Mar 2021 09:48:00 -0000 * Adhemerval Zanella via Libc-alpha: > diff --git a/manual/llio.texi b/manual/llio.texi > index c0a53e1a6e..ceb18ac89a 100644 > --- a/manual/llio.texi > +++ b/manual/llio.texi > @@ -284,6 +284,44 @@ of trying to close its underlying file descriptor wi= th @code{close}. > This flushes any buffered output and updates the stream object to > indicate that it is closed. > =20 > +@deftypefun int close_range (unsigned int @var{lowfd}, unsigned int @var= {maxfd}, int @var{flags}) > +@standards{Linux, unistd.h} > +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{@acsfd{}}} > +@c This is a syscall for Linux v5.9. There is no fallback emulation for > +@c older kernels. > + > +The function @code{clode_range} closes the file descriptor from @var{low= fd} > +to @var{maxfd} (inclusive). This function is similar to call @code{clos= e} in > +specified file descriptor range depending on the @var{flags}. > + > +The @var{flags} add options on how the files are closes. Linux currentl= y > +supports:. Spurios =E2=80=9C.=E2=80=9D at end of line. > +@vtable @code > +@item CLOSE_RANGE_UNSHARE > +Unshare the file descriptor table before closing file descriptors. > +@end vtable > + > +The normal return value from @code{close_range} is @math{0}; a value > +of @math{-1} is returned in case of failure. The following @code{errno}= error > +conditions are defined for this function: > + > +@table @code > +@item EINVAL > +The @var{lowfd} value is larger than @var{maxfd} or an unsupported @var{= flags} > +is used. > + > +@item ENOMEM > +Either there is not enough memory for the operation, or the process is > +out of address space. > + > +@item EMFILE > +The process has too many files open. > +The maximum number of file descriptors is controlled by the > +@end table > +@end deftypefun The ENOMEM and EMFILE descriptions are not really clear in this context. Are these failures only possible with CLOSE_RANGE_UNSHARE? It may make sense to mention the lack of emulation (so that callers have to be aware of ENOSYS for the time being) and that the function is specific to Linux. Based on the description, it is not clear what happens if the range contains closed descriptors. Maybe mention that already-closed descriptors are simply skipped? > index c35f783e2a..e2a6fa763f 100644 > --- a/sysdeps/unix/sysv/linux/Versions > +++ b/sysdeps/unix/sysv/linux/Versions > @@ -169,6 +169,9 @@ libc { > } > GLIBC_2.32 { > } > + GLIBC_2.33 { > + close_range; > + } Needs to be GLIBC_2.34 now. Also the copyright year needs adjusting. > diff --git a/sysdeps/unix/sysv/linux/bits/unistd_ext.h b/sysdeps/unix/sys= v/linux/bits/unistd_ext.h > index c315cc5cb8..799c59512c 100644 > --- a/sysdeps/unix/sysv/linux/bits/unistd_ext.h > +++ b/sysdeps/unix/sysv/linux/bits/unistd_ext.h > @@ -33,4 +33,15 @@ > not detached and has not been joined. */ > extern __pid_t gettid (void) __THROW; > =20 > +/* Unshare the file descriptor table before closing file descriptors. *= / > +#define CLOSE_RANGE_UNSHARE (1U << 1) Needs to be indented. Please include if available. It's a separate header so that we can use it. I think if you do that, the consistency check won't add much value because it can't really test anything. > +/* Close all file descriptors in the range FD up to MAX_FD. The flag FL= AGS > + are define by the CLOSE_RANGE prefix. This function behaves like clo= se > + on the range, but in a fail-safe where it will either fail and not cl= ose > + any file descriptor or close all of them. Returns 0 on successor or = -1 > + for failure (and sets errno accordingly). */ > +extern int close_range (unsigned int __fd, unsigned int __max_fd, > +=09=09=09int __flags) __THROW; > + > #endif Maybe add /* __USE_GNU */ to the #endif, given that it's now at a distance from the #ifdef. > diff --git a/sysdeps/unix/sysv/linux/tst-close_range.c b/sysdeps/unix/sys= v/linux/tst-close_range.c > new file mode 100644 > index 0000000000..131cf27c72 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-close_range.c > @@ -0,0 +1,222 @@ > +#define NFDS 100 > + > +static int > +open_multiple_temp_files (void) > +{ > + /* Check if the temporary file descriptor has no no gaps. */ > + int lowfd =3D xopen ("/dev/null", O_RDONLY, 0600); > + for (int i =3D 1; i <=3D NFDS; i++) > + TEST_COMPARE (xopen ("/dev/null", O_RDONLY, 0600), > +=09=09 lowfd + i); > + return lowfd; > +} Okay, this ensures that there are no gaps. > + > +static void > +close_range_test_common (int lowfd, unsigned int flags) > +{ > + const int maximum_fd =3D lowfd + NFDS; > + const int half_fd =3D maximum_fd / 2; > + const int gap_1 =3D maximum_fd - 8; I think half_fd should be should be lowfd + NFDS / 2. > +_Noreturn static int > +close_range_test_fn (void *arg) > +{ > + int lowfd =3D (int) ((uintptr_t) arg); > + close_range_test_common (lowfd, 0); > + exit (EXIT_SUCCESS); > +} > + > +/* Check if a clone_range on a subprocess created with CLONE_FILES close > + the shared file descriptor table entries in the parent. */ > +static void > +close_range_test_subprocess (void) > +{ > + struct support_descriptors *descrs =3D support_descriptors_list (); > + > + /* Check if the temporary file descriptor has no no gaps. */ > + int lowfd =3D open_multiple_temp_files (); > + > + enum { stack_size =3D 4096 }; > + DEFINE_STACK (stack, stack_size); > + pid_t pid =3D xclone (close_range_test_fn, (void*) (uintptr_t) lowfd, = stack, > +=09=09 stack_size, CLONE_FILES | SIGCHLD); stack_size is too small, I think. Future error checking (possible with clone3) would lead to failures. > +static void > +close_range_unshare_test (void) > +{ > + struct support_descriptors *descrs1 =3D support_descriptors_list (); > + > + /* Check if the temporary file descriptor has no no gaps. */ > + int lowfd =3D open_multiple_temp_files (); > + > + struct support_descriptors *descrs2 =3D support_descriptors_list (); > + > + enum { stack_size =3D 4096 }; > + DEFINE_STACK (stack, stack_size); > + pid_t pid =3D xclone (close_range_unshare_test_fn, (void*) (uintptr_t)= lowfd, > +=09=09 stack, stack_size, CLONE_FILES | SIGCHLD); Likewise. Thanks, Florian