Message ID | CAMe9rOqQL07x-QBAHhgiGZh=s98LJ35Mrec4QKfV18SohnRQnA@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 19/07/2017 18:53, H.J. Lu wrote: > On Tue, Jul 18, 2017 at 6:52 AM, Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> On 18/07/2017 10:44, H.J. Lu wrote: >>> This should work for Linux. >>> >>> Thanks. >>> >> >> Now with a proper patch: >> >> [PATCH] tunables: Use direct syscall for access (BZ#21744) >> >> The function maybe_enable_malloc_check, which is called by >> __tunables_init, calls __access_noerrno. It isn't problem when >> symbol is is in ld.so, which has a special version of __access_noerrno >> without stack protector. But when glibc is built with stack protector, >> maybe_enable_malloc_check in libc.a can't call the regular version of >> __access_noerrno with stack protector. >> >> This patch changes how Linux defines the __access_noerrno to be an >> inline call instead and thus preventing defining different build >> rules for ld/static and shared. >> >> H.J. Lu <hongjiu.lu@intel.com> >> Adhemerval Zanella <adhemerval.zanella@linaro.org> > > Missing [BZ#21744] Ack. > >> * elf/dl-tunables.c: Include not-errno.h header. >> * include/unistd.h (__access_noerrno): Remove definition. >> * sysdeps/unix/sysv/linux/access.c (__access_noerrno): Likewise. >> * sysdeps/generic/not-errno.h: New file. >> * sysdeps/unix/sysv/linux/not-errno.h: Likewise. >> --- >> ChangeLog | 9 +++++++++ >> elf/dl-tunables.c | 2 ++ >> include/unistd.h | 7 ------- >> sysdeps/generic/not-errno.h | 19 +++++++++++++++++++ >> sysdeps/unix/sysv/linux/access.c | 15 --------------- >> sysdeps/unix/sysv/linux/not-errno.h | 30 ++++++++++++++++++++++++++++++ >> 6 files changed, 60 insertions(+), 22 deletions(-) >> create mode 100644 sysdeps/generic/not-errno.h >> create mode 100644 sysdeps/unix/sysv/linux/not-errno.h >> >> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c >> index 44c160c..231fb8c 100644 >> --- a/elf/dl-tunables.c >> +++ b/elf/dl-tunables.c >> @@ -29,6 +29,8 @@ >> #define TUNABLES_INTERNAL 1 >> #include "dl-tunables.h" >> >> +#include <not-errno.h> >> + >> #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring >> # define GLIBC_TUNABLES "GLIBC_TUNABLES" >> #endif >> diff --git a/include/unistd.h b/include/unistd.h >> index 5b2a414..7f1c2cc 100644 >> --- a/include/unistd.h >> +++ b/include/unistd.h >> @@ -182,12 +182,5 @@ extern int __getlogin_r_loginuid (char *name, size_t namesize) >> # include <dl-unistd.h> >> # endif >> >> -# if IS_IN (rtld) || !defined SHARED >> -/* __access variant that does not set errno. Used in very early initialization >> - code in libc.a and ld.so. It follows access return semantics (zero for >> - sucess otherwise a value different than 0). */ >> -extern __typeof (__access) __access_noerrno attribute_hidden; >> -# endif >> - >> # endif >> #endif >> diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h >> new file mode 100644 >> index 0000000..2aac095 >> --- /dev/null >> +++ b/sysdeps/generic/not-errno.h >> @@ -0,0 +1,19 @@ >> +/* Syscall wrapper that do not set errno. Generic version. >> + Copyright (C) 2017 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 >> + <http://www.gnu.org/licenses/>. */ >> + >> +extern __typeof (__access) __access_noerrno attribute_hidden; >> diff --git a/sysdeps/unix/sysv/linux/access.c b/sysdeps/unix/sysv/linux/access.c >> index 67e69bd..366b6b6 100644 >> --- a/sysdeps/unix/sysv/linux/access.c >> +++ b/sysdeps/unix/sysv/linux/access.c >> @@ -21,21 +21,6 @@ >> #include <sysdep-cancel.h> >> >> int >> -__access_noerrno (const char *file, int type) >> -{ >> - int res; >> - INTERNAL_SYSCALL_DECL (err); >> -#ifdef __NR_access >> - res = INTERNAL_SYSCALL_CALL (access, err, file, type); >> -#else >> - res = INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type); >> -#endif >> - if (INTERNAL_SYSCALL_ERROR_P (res, err)) >> - return INTERNAL_SYSCALL_ERRNO (res, err); >> - return 0; >> -} >> - >> -int >> __access (const char *file, int type) >> { >> #ifdef __NR_access >> diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h >> new file mode 100644 >> index 0000000..4957038 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/not-errno.h >> @@ -0,0 +1,30 @@ >> +/* Syscall wrapper that do not set errno. Linux version. >> + Copyright (C) 2017 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 >> + <http://www.gnu.org/licenses/>. */ >> + >> +/* 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) >> +{ >> +#ifdef __NR_access >> + return INTERNAL_SYSCALL_CALL (access, pathname, mode); >> +#else >> + return INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); >> +#endif >> +} > > These are wrong. INTERNAL_SYSCALL_CALL needs a dummy err. > This seems to work: > > diff --git a/sysdeps/unix/sysv/linux/not-errno.h > b/sysdeps/unix/sysv/linux/not-errno.h > index 4957038160..a03c399f95 100644 > --- a/sysdeps/unix/sysv/linux/not-errno.h > +++ b/sysdeps/unix/sysv/linux/not-errno.h > @@ -23,8 +23,8 @@ static inline int > __access_noerrno (const char *pathname, int mode) > { > #ifdef __NR_access > - return INTERNAL_SYSCALL_CALL (access, pathname, mode); > + return INTERNAL_SYSCALL_CALL (access, err, pathname, mode); > #else > - return INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); > + return INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type); > #endif > } > Right, so we will need the 'INTERNAL_SYSCALL_DECL (err);' before as well.
On Wed, Jul 19, 2017 at 5:56 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > On 19/07/2017 18:53, H.J. Lu wrote: >> On Tue, Jul 18, 2017 at 6:52 AM, Adhemerval Zanella >> <adhemerval.zanella@linaro.org> wrote: >>> >>> >>> On 18/07/2017 10:44, H.J. Lu wrote: >>>> This should work for Linux. >>>> >>>> Thanks. >>>> >>> >>> Now with a proper patch: >>> >>> [PATCH] tunables: Use direct syscall for access (BZ#21744) >>> >>> The function maybe_enable_malloc_check, which is called by >>> __tunables_init, calls __access_noerrno. It isn't problem when >>> symbol is is in ld.so, which has a special version of __access_noerrno >>> without stack protector. But when glibc is built with stack protector, >>> maybe_enable_malloc_check in libc.a can't call the regular version of >>> __access_noerrno with stack protector. >>> >>> This patch changes how Linux defines the __access_noerrno to be an >>> inline call instead and thus preventing defining different build >>> rules for ld/static and shared. >>> >>> H.J. Lu <hongjiu.lu@intel.com> >>> Adhemerval Zanella <adhemerval.zanella@linaro.org> >> >> Missing [BZ#21744] > > Ack. > >> >>> * elf/dl-tunables.c: Include not-errno.h header. >>> * include/unistd.h (__access_noerrno): Remove definition. >>> * sysdeps/unix/sysv/linux/access.c (__access_noerrno): Likewise. >>> * sysdeps/generic/not-errno.h: New file. >>> * sysdeps/unix/sysv/linux/not-errno.h: Likewise. >>> --- >>> ChangeLog | 9 +++++++++ >>> elf/dl-tunables.c | 2 ++ >>> include/unistd.h | 7 ------- >>> sysdeps/generic/not-errno.h | 19 +++++++++++++++++++ >>> sysdeps/unix/sysv/linux/access.c | 15 --------------- >>> sysdeps/unix/sysv/linux/not-errno.h | 30 ++++++++++++++++++++++++++++++ >>> 6 files changed, 60 insertions(+), 22 deletions(-) >>> create mode 100644 sysdeps/generic/not-errno.h >>> create mode 100644 sysdeps/unix/sysv/linux/not-errno.h >>> >>> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c >>> index 44c160c..231fb8c 100644 >>> --- a/elf/dl-tunables.c >>> +++ b/elf/dl-tunables.c >>> @@ -29,6 +29,8 @@ >>> #define TUNABLES_INTERNAL 1 >>> #include "dl-tunables.h" >>> >>> +#include <not-errno.h> >>> + >>> #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring >>> # define GLIBC_TUNABLES "GLIBC_TUNABLES" >>> #endif >>> diff --git a/include/unistd.h b/include/unistd.h >>> index 5b2a414..7f1c2cc 100644 >>> --- a/include/unistd.h >>> +++ b/include/unistd.h >>> @@ -182,12 +182,5 @@ extern int __getlogin_r_loginuid (char *name, size_t namesize) >>> # include <dl-unistd.h> >>> # endif >>> >>> -# if IS_IN (rtld) || !defined SHARED >>> -/* __access variant that does not set errno. Used in very early initialization >>> - code in libc.a and ld.so. It follows access return semantics (zero for >>> - sucess otherwise a value different than 0). */ >>> -extern __typeof (__access) __access_noerrno attribute_hidden; >>> -# endif >>> - >>> # endif >>> #endif >>> diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h >>> new file mode 100644 >>> index 0000000..2aac095 >>> --- /dev/null >>> +++ b/sysdeps/generic/not-errno.h >>> @@ -0,0 +1,19 @@ >>> +/* Syscall wrapper that do not set errno. Generic version. >>> + Copyright (C) 2017 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 >>> + <http://www.gnu.org/licenses/>. */ >>> + >>> +extern __typeof (__access) __access_noerrno attribute_hidden; >>> diff --git a/sysdeps/unix/sysv/linux/access.c b/sysdeps/unix/sysv/linux/access.c >>> index 67e69bd..366b6b6 100644 >>> --- a/sysdeps/unix/sysv/linux/access.c >>> +++ b/sysdeps/unix/sysv/linux/access.c >>> @@ -21,21 +21,6 @@ >>> #include <sysdep-cancel.h> >>> >>> int >>> -__access_noerrno (const char *file, int type) >>> -{ >>> - int res; >>> - INTERNAL_SYSCALL_DECL (err); >>> -#ifdef __NR_access >>> - res = INTERNAL_SYSCALL_CALL (access, err, file, type); >>> -#else >>> - res = INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type); >>> -#endif >>> - if (INTERNAL_SYSCALL_ERROR_P (res, err)) >>> - return INTERNAL_SYSCALL_ERRNO (res, err); >>> - return 0; >>> -} >>> - >>> -int >>> __access (const char *file, int type) >>> { >>> #ifdef __NR_access >>> diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h >>> new file mode 100644 >>> index 0000000..4957038 >>> --- /dev/null >>> +++ b/sysdeps/unix/sysv/linux/not-errno.h >>> @@ -0,0 +1,30 @@ >>> +/* Syscall wrapper that do not set errno. Linux version. >>> + Copyright (C) 2017 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 >>> + <http://www.gnu.org/licenses/>. */ >>> + >>> +/* 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) >>> +{ >>> +#ifdef __NR_access >>> + return INTERNAL_SYSCALL_CALL (access, pathname, mode); >>> +#else >>> + return INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); >>> +#endif >>> +} >> >> These are wrong. INTERNAL_SYSCALL_CALL needs a dummy err. >> This seems to work: >> >> diff --git a/sysdeps/unix/sysv/linux/not-errno.h >> b/sysdeps/unix/sysv/linux/not-errno.h >> index 4957038160..a03c399f95 100644 >> --- a/sysdeps/unix/sysv/linux/not-errno.h >> +++ b/sysdeps/unix/sysv/linux/not-errno.h >> @@ -23,8 +23,8 @@ static inline int >> __access_noerrno (const char *pathname, int mode) >> { >> #ifdef __NR_access >> - return INTERNAL_SYSCALL_CALL (access, pathname, mode); >> + return INTERNAL_SYSCALL_CALL (access, err, pathname, mode); >> #else >> - return INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); >> + return INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type); >> #endif >> } >> > > Right, so we will need the 'INTERNAL_SYSCALL_DECL (err);' before as well. It compiles fine without "INTERNAL_SYSCALL_DECL (err);".
diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h index 4957038160..a03c399f95 100644 --- a/sysdeps/unix/sysv/linux/not-errno.h +++ b/sysdeps/unix/sysv/linux/not-errno.h @@ -23,8 +23,8 @@ static inline int __access_noerrno (const char *pathname, int mode) { #ifdef __NR_access - return INTERNAL_SYSCALL_CALL (access, pathname, mode); + return INTERNAL_SYSCALL_CALL (access, err, pathname, mode); #else - return INTERNAL_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); + return INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type); #endif }