This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Don't all __access_noerrno with stack protector from __tunables_init [BZ #21744]
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);".
--
H.J.