| Message ID | 72e13315-f067-835d-8fbb-1f8683933881@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove __access_noerrno | expand |
Hello, Joseph Myers, le jeu. 23 nov. 2023 18:04:54 +0000, a ecrit: > A recent commit, apparently commit > 6c6fce572fb8f583f14d898e54fd7d25ae91cf56 "elf: Remove /etc/suid-debug > support", resulted in localplt failures for i686-gnu and x86_64-gnu: > > Missing required PLT reference: ld.so: __access_noerrno > > After that commit, __access_noerrno is actually no longer used at all. > So rather than just removing the localplt expectation for that symbol > for Hurd, completely remove all definitions of and references to that > symbol. > > Tested for x86_64, and with build-many-glibcs.py for i686-gnu and > x86_64-gnu. Looks good, thanks! > Note: just above the changes to sysdeps/mach/hurd/dl-sysdep.c, there > remain a comment and definition in that file: > > /* This function is called by the dynamic linker (rtld.c) to check > whether debugging malloc is allowed even for SUID binaries. This > stub will always fail, which means that malloc-debugging is always > disabled for SUID binaries. */ > check_no_hidden(__access); > int weak_function > __access (const char *file, int type) > { > return __hurd_fail (ENOSYS); > } > > That comment is certainly inaccurate after /etc/suid-debug support was > removed. But __access is also used in rtld.c for /etc/ld.so.preload. > So does that __access use also need this special dummy definition on > Hurd, in which case the comment should be changed to say that > /etc/ld.so.preload is unsupported on Hurd, Yes. > or would the normal Hurd __access work without needing a special > dl-sysdep.c definition? Mmm, in principle it should be working, since it doesn't actually use fds (__access just passes AT_FDCWD to __accessat), but access uses real ids, not effective ids, and that is probably not implemented yet in dl-sysdep.c. Apparently malloc-debugging wasn't seen important enough for the writer of that comment to implement __access, but ld.so.preload support would be useful, so that would become a TODO, yes. Samuel > diff --git a/io/access.c b/io/access.c > index 29b8fdce89..06a3900a45 100644 > --- a/io/access.c > +++ b/io/access.c > @@ -19,13 +19,6 @@ > #include <stddef.h> > #include <unistd.h> > > -/* Test for access to FILE without setting errno. */ > -int > -__access_noerrno (const char *file, int type) > -{ > - return -1; > -} > - > /* Test for access to FILE. */ > int > __access (const char *file, int type) > diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h > index 303f8a68a8..16ae34b42f 100644 > --- a/sysdeps/generic/not-errno.h > +++ b/sysdeps/generic/not-errno.h > @@ -16,6 +16,4 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > -extern __typeof (__access) __access_noerrno attribute_hidden; > - > extern __typeof (__kill) __kill_noerrno attribute_hidden; > diff --git a/sysdeps/mach/hurd/Versions b/sysdeps/mach/hurd/Versions > index 20ac0067c7..17592d3569 100644 > --- a/sysdeps/mach/hurd/Versions > +++ b/sysdeps/mach/hurd/Versions > @@ -15,7 +15,7 @@ libc { > } > GLIBC_PRIVATE { > # Functions shared with the dynamic linker > - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; > + __access; __libc_read; __libc_write; __libc_lseek64; > __close_nocancel; > __open_nocancel; > __read_nocancel; __pread64_nocancel; > @@ -56,7 +56,7 @@ ld { > _dl_init_first; > > # functions that must be shared with libc > - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; > + __access; __libc_read; __libc_write; __libc_lseek64; > __close_nocancel; > __open_nocancel; > __read_nocancel; __pread64_nocancel; > diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c > index 9e8b9014a7..87a15e5a8b 100644 > --- a/sysdeps/mach/hurd/access.c > +++ b/sysdeps/mach/hurd/access.c > @@ -19,16 +19,6 @@ > #include <unistd.h> > #include <fcntl.h> > > -/* Test for access to FILE by our real user and group IDs without setting > - errno. This may be unsafe to run during initialization of tunables > - since access_common calls __hurd_file_name_lookup, which calls > - __hurd_file_name_lookup_retry, which can set errno. */ > -int > -__access_noerrno (const char *file, int type) > -{ > - return __faccessat_noerrno (AT_FDCWD, file, type, 0); > -} > - > /* Test for access to FILE by our real user and group IDs. */ > int > __access (const char *file, int type) > diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c > index 6994f8f7b6..8dda4a1f83 100644 > --- a/sysdeps/mach/hurd/dl-sysdep.c > +++ b/sysdeps/mach/hurd/dl-sysdep.c > @@ -552,12 +552,6 @@ __access (const char *file, int type) > { > return __hurd_fail (ENOSYS); > } > -check_no_hidden(__access_noerrno); > -int weak_function > -__access_noerrno (const char *file, int type) > -{ > - return -1; > -} > > int > __rtld_execve (const char *file_name, char *const argv[], > diff --git a/sysdeps/mach/hurd/i386/localplt.data b/sysdeps/mach/hurd/i386/localplt.data > index 89b124db9f..b7882d0fbd 100644 > --- a/sysdeps/mach/hurd/i386/localplt.data > +++ b/sysdeps/mach/hurd/i386/localplt.data > @@ -27,7 +27,6 @@ ld.so: __mmap > ld.so: __fstat64 > ld.so: __stat64 > ld.so: __access > -ld.so: __access_noerrno > ld.so: __getpid > ld.so: __getcwd > ld.so: _exit ? > diff --git a/sysdeps/mach/hurd/not-errno.h b/sysdeps/mach/hurd/not-errno.h > deleted file mode 100644 > index 956b065974..0000000000 > --- a/sysdeps/mach/hurd/not-errno.h > +++ /dev/null > @@ -1,21 +0,0 @@ > -/* Syscall wrapper that do not set errno. Generic version. > - Copyright (C) 2017-2023 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/>. */ > - > -/* The __access_noerrno stub mustn't be hidden in ld.so on Hurd since it needs > - to be preempted by __access_noerrno from libc.so after bootstrap. */ > -extern __typeof (__access) __access_noerrno; > diff --git a/sysdeps/mach/hurd/x86_64/localplt.data b/sysdeps/mach/hurd/x86_64/localplt.data > index c38821e2fa..b7689b5cb0 100644 > --- a/sysdeps/mach/hurd/x86_64/localplt.data > +++ b/sysdeps/mach/hurd/x86_64/localplt.data > @@ -26,7 +26,6 @@ ld.so: __mmap > ld.so: __fstat64 > ld.so: __stat64 > ld.so: __access > -ld.so: __access_noerrno > ld.so: __getpid > ld.so: __getcwd > ld.so: _exit ? > diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h > index 9100bde124..e55a7c4698 100644 > --- a/sysdeps/unix/sysv/linux/not-errno.h > +++ b/sysdeps/unix/sysv/linux/not-errno.h > @@ -19,23 +19,6 @@ > #include <sysdep.h> > #include <fcntl.h> > > -/* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c) > - and to avoid having to build/use multiple versions if stack protection > - in enabled it is defined as inline. */ > -static inline int > -__access_noerrno (const char *pathname, int mode) > -{ > - int res; > -#ifdef __NR_access > - res = INTERNAL_SYSCALL_CALL (access, pathname, mode); > -#else > - res = INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, pathname, mode); > -#endif > - if (INTERNAL_SYSCALL_ERROR_P (res)) > - return INTERNAL_SYSCALL_ERRNO (res); > - return 0; > -} > - > static inline int > __kill_noerrno (pid_t pid, int sig) > { > > -- > Joseph S. Myers > joseph@codesourcery.com >
On Thu, 23 Nov 2023, Samuel Thibault wrote: > > That comment is certainly inaccurate after /etc/suid-debug support was > > removed. But __access is also used in rtld.c for /etc/ld.so.preload. > > So does that __access use also need this special dummy definition on > > Hurd, in which case the comment should be changed to say that > > /etc/ld.so.preload is unsupported on Hurd, > > Yes. Thanks, I've committed this patch version that also updates that comment. Remove __access_noerrno A recent commit, apparently commit 6c6fce572fb8f583f14d898e54fd7d25ae91cf56 "elf: Remove /etc/suid-debug support", resulted in localplt failures for i686-gnu and x86_64-gnu: Missing required PLT reference: ld.so: __access_noerrno After that commit, __access_noerrno is actually no longer used at all. So rather than just removing the localplt expectation for that symbol for Hurd, completely remove all definitions of and references to that symbol. Tested for x86_64, and with build-many-glibcs.py for i686-gnu and x86_64-gnu. diff --git a/io/access.c b/io/access.c index 29b8fdce89..06a3900a45 100644 --- a/io/access.c +++ b/io/access.c @@ -19,13 +19,6 @@ #include <stddef.h> #include <unistd.h> -/* Test for access to FILE without setting errno. */ -int -__access_noerrno (const char *file, int type) -{ - return -1; -} - /* Test for access to FILE. */ int __access (const char *file, int type) diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h index 303f8a68a8..16ae34b42f 100644 --- a/sysdeps/generic/not-errno.h +++ b/sysdeps/generic/not-errno.h @@ -16,6 +16,4 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -extern __typeof (__access) __access_noerrno attribute_hidden; - extern __typeof (__kill) __kill_noerrno attribute_hidden; diff --git a/sysdeps/mach/hurd/Versions b/sysdeps/mach/hurd/Versions index 20ac0067c7..17592d3569 100644 --- a/sysdeps/mach/hurd/Versions +++ b/sysdeps/mach/hurd/Versions @@ -15,7 +15,7 @@ libc { } GLIBC_PRIVATE { # Functions shared with the dynamic linker - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; + __access; __libc_read; __libc_write; __libc_lseek64; __close_nocancel; __open_nocancel; __read_nocancel; __pread64_nocancel; @@ -56,7 +56,7 @@ ld { _dl_init_first; # functions that must be shared with libc - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; + __access; __libc_read; __libc_write; __libc_lseek64; __close_nocancel; __open_nocancel; __read_nocancel; __pread64_nocancel; diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c index 9e8b9014a7..87a15e5a8b 100644 --- a/sysdeps/mach/hurd/access.c +++ b/sysdeps/mach/hurd/access.c @@ -19,16 +19,6 @@ #include <unistd.h> #include <fcntl.h> -/* Test for access to FILE by our real user and group IDs without setting - errno. This may be unsafe to run during initialization of tunables - since access_common calls __hurd_file_name_lookup, which calls - __hurd_file_name_lookup_retry, which can set errno. */ -int -__access_noerrno (const char *file, int type) -{ - return __faccessat_noerrno (AT_FDCWD, file, type, 0); -} - /* Test for access to FILE by our real user and group IDs. */ int __access (const char *file, int type) diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index 6994f8f7b6..38e55acc43 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -542,22 +542,15 @@ __stat64 (const char *file, struct stat64 *buf) } libc_hidden_def (__stat64) -/* This function is called by the dynamic linker (rtld.c) to check - whether debugging malloc is allowed even for SUID binaries. This - stub will always fail, which means that malloc-debugging is always - disabled for SUID binaries. */ +/* This function is called by the dynamic linker (rtld.c) to check for + existence of /etc/ld.so.preload. This stub will always fail, which + means that /etc/ld.so.preload is unsupported. */ check_no_hidden(__access); int weak_function __access (const char *file, int type) { return __hurd_fail (ENOSYS); } -check_no_hidden(__access_noerrno); -int weak_function -__access_noerrno (const char *file, int type) -{ - return -1; -} int __rtld_execve (const char *file_name, char *const argv[], diff --git a/sysdeps/mach/hurd/i386/localplt.data b/sysdeps/mach/hurd/i386/localplt.data index 89b124db9f..b7882d0fbd 100644 --- a/sysdeps/mach/hurd/i386/localplt.data +++ b/sysdeps/mach/hurd/i386/localplt.data @@ -27,7 +27,6 @@ ld.so: __mmap ld.so: __fstat64 ld.so: __stat64 ld.so: __access -ld.so: __access_noerrno ld.so: __getpid ld.so: __getcwd ld.so: _exit ? diff --git a/sysdeps/mach/hurd/not-errno.h b/sysdeps/mach/hurd/not-errno.h deleted file mode 100644 index 956b065974..0000000000 --- a/sysdeps/mach/hurd/not-errno.h +++ /dev/null @@ -1,21 +0,0 @@ -/* Syscall wrapper that do not set errno. Generic version. - Copyright (C) 2017-2023 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/>. */ - -/* The __access_noerrno stub mustn't be hidden in ld.so on Hurd since it needs - to be preempted by __access_noerrno from libc.so after bootstrap. */ -extern __typeof (__access) __access_noerrno; diff --git a/sysdeps/mach/hurd/x86_64/localplt.data b/sysdeps/mach/hurd/x86_64/localplt.data index c38821e2fa..b7689b5cb0 100644 --- a/sysdeps/mach/hurd/x86_64/localplt.data +++ b/sysdeps/mach/hurd/x86_64/localplt.data @@ -26,7 +26,6 @@ ld.so: __mmap ld.so: __fstat64 ld.so: __stat64 ld.so: __access -ld.so: __access_noerrno ld.so: __getpid ld.so: __getcwd ld.so: _exit ? diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h index 9100bde124..e55a7c4698 100644 --- a/sysdeps/unix/sysv/linux/not-errno.h +++ b/sysdeps/unix/sysv/linux/not-errno.h @@ -19,23 +19,6 @@ #include <sysdep.h> #include <fcntl.h> -/* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c) - and to avoid having to build/use multiple versions if stack protection - in enabled it is defined as inline. */ -static inline int -__access_noerrno (const char *pathname, int mode) -{ - int res; -#ifdef __NR_access - res = INTERNAL_SYSCALL_CALL (access, pathname, mode); -#else - res = INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, pathname, mode); -#endif - if (INTERNAL_SYSCALL_ERROR_P (res)) - return INTERNAL_SYSCALL_ERRNO (res); - return 0; -} - static inline int __kill_noerrno (pid_t pid, int sig) {
diff --git a/io/access.c b/io/access.c index 29b8fdce89..06a3900a45 100644 --- a/io/access.c +++ b/io/access.c @@ -19,13 +19,6 @@ #include <stddef.h> #include <unistd.h> -/* Test for access to FILE without setting errno. */ -int -__access_noerrno (const char *file, int type) -{ - return -1; -} - /* Test for access to FILE. */ int __access (const char *file, int type) diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h index 303f8a68a8..16ae34b42f 100644 --- a/sysdeps/generic/not-errno.h +++ b/sysdeps/generic/not-errno.h @@ -16,6 +16,4 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -extern __typeof (__access) __access_noerrno attribute_hidden; - extern __typeof (__kill) __kill_noerrno attribute_hidden; diff --git a/sysdeps/mach/hurd/Versions b/sysdeps/mach/hurd/Versions index 20ac0067c7..17592d3569 100644 --- a/sysdeps/mach/hurd/Versions +++ b/sysdeps/mach/hurd/Versions @@ -15,7 +15,7 @@ libc { } GLIBC_PRIVATE { # Functions shared with the dynamic linker - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; + __access; __libc_read; __libc_write; __libc_lseek64; __close_nocancel; __open_nocancel; __read_nocancel; __pread64_nocancel; @@ -56,7 +56,7 @@ ld { _dl_init_first; # functions that must be shared with libc - __access; __access_noerrno; __libc_read; __libc_write; __libc_lseek64; + __access; __libc_read; __libc_write; __libc_lseek64; __close_nocancel; __open_nocancel; __read_nocancel; __pread64_nocancel; diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c index 9e8b9014a7..87a15e5a8b 100644 --- a/sysdeps/mach/hurd/access.c +++ b/sysdeps/mach/hurd/access.c @@ -19,16 +19,6 @@ #include <unistd.h> #include <fcntl.h> -/* Test for access to FILE by our real user and group IDs without setting - errno. This may be unsafe to run during initialization of tunables - since access_common calls __hurd_file_name_lookup, which calls - __hurd_file_name_lookup_retry, which can set errno. */ -int -__access_noerrno (const char *file, int type) -{ - return __faccessat_noerrno (AT_FDCWD, file, type, 0); -} - /* Test for access to FILE by our real user and group IDs. */ int __access (const char *file, int type) diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c index 6994f8f7b6..8dda4a1f83 100644 --- a/sysdeps/mach/hurd/dl-sysdep.c +++ b/sysdeps/mach/hurd/dl-sysdep.c @@ -552,12 +552,6 @@ __access (const char *file, int type) { return __hurd_fail (ENOSYS); } -check_no_hidden(__access_noerrno); -int weak_function -__access_noerrno (const char *file, int type) -{ - return -1; -} int __rtld_execve (const char *file_name, char *const argv[], diff --git a/sysdeps/mach/hurd/i386/localplt.data b/sysdeps/mach/hurd/i386/localplt.data index 89b124db9f..b7882d0fbd 100644 --- a/sysdeps/mach/hurd/i386/localplt.data +++ b/sysdeps/mach/hurd/i386/localplt.data @@ -27,7 +27,6 @@ ld.so: __mmap ld.so: __fstat64 ld.so: __stat64 ld.so: __access -ld.so: __access_noerrno ld.so: __getpid ld.so: __getcwd ld.so: _exit ? diff --git a/sysdeps/mach/hurd/not-errno.h b/sysdeps/mach/hurd/not-errno.h deleted file mode 100644 index 956b065974..0000000000 --- a/sysdeps/mach/hurd/not-errno.h +++ /dev/null @@ -1,21 +0,0 @@ -/* Syscall wrapper that do not set errno. Generic version. - Copyright (C) 2017-2023 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/>. */ - -/* The __access_noerrno stub mustn't be hidden in ld.so on Hurd since it needs - to be preempted by __access_noerrno from libc.so after bootstrap. */ -extern __typeof (__access) __access_noerrno; diff --git a/sysdeps/mach/hurd/x86_64/localplt.data b/sysdeps/mach/hurd/x86_64/localplt.data index c38821e2fa..b7689b5cb0 100644 --- a/sysdeps/mach/hurd/x86_64/localplt.data +++ b/sysdeps/mach/hurd/x86_64/localplt.data @@ -26,7 +26,6 @@ ld.so: __mmap ld.so: __fstat64 ld.so: __stat64 ld.so: __access -ld.so: __access_noerrno ld.so: __getpid ld.so: __getcwd ld.so: _exit ? diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h index 9100bde124..e55a7c4698 100644 --- a/sysdeps/unix/sysv/linux/not-errno.h +++ b/sysdeps/unix/sysv/linux/not-errno.h @@ -19,23 +19,6 @@ #include <sysdep.h> #include <fcntl.h> -/* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c) - and to avoid having to build/use multiple versions if stack protection - in enabled it is defined as inline. */ -static inline int -__access_noerrno (const char *pathname, int mode) -{ - int res; -#ifdef __NR_access - res = INTERNAL_SYSCALL_CALL (access, pathname, mode); -#else - res = INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, pathname, mode); -#endif - if (INTERNAL_SYSCALL_ERROR_P (res)) - return INTERNAL_SYSCALL_ERRNO (res); - return 0; -} - static inline int __kill_noerrno (pid_t pid, int sig) {
A recent commit, apparently commit 6c6fce572fb8f583f14d898e54fd7d25ae91cf56 "elf: Remove /etc/suid-debug support", resulted in localplt failures for i686-gnu and x86_64-gnu: Missing required PLT reference: ld.so: __access_noerrno After that commit, __access_noerrno is actually no longer used at all. So rather than just removing the localplt expectation for that symbol for Hurd, completely remove all definitions of and references to that symbol. Tested for x86_64, and with build-many-glibcs.py for i686-gnu and x86_64-gnu. Note: just above the changes to sysdeps/mach/hurd/dl-sysdep.c, there remain a comment and definition in that file: /* This function is called by the dynamic linker (rtld.c) to check whether debugging malloc is allowed even for SUID binaries. This stub will always fail, which means that malloc-debugging is always disabled for SUID binaries. */ check_no_hidden(__access); int weak_function __access (const char *file, int type) { return __hurd_fail (ENOSYS); } That comment is certainly inaccurate after /etc/suid-debug support was removed. But __access is also used in rtld.c for /etc/ld.so.preload. So does that __access use also need this special dummy definition on Hurd, in which case the comment should be changed to say that /etc/ld.so.preload is unsupported on Hurd, or would the normal Hurd __access work without needing a special dl-sysdep.c definition?