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 19/07/2017 22:11, H.J. Lu wrote:
> 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);".
It will fail for architectures that actually define 'err' to something other
than empty 'do { } while (0)' (for instance sparc and tile). Below it is an
updated patch:
--
[BZ #21744]
* 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.
---
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..888bde0
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/not-errno.h
@@ -0,0 +1,31 @@
+/* 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)
+{
+ INTERNAL_SYSCALL_DECL (err);
+#ifdef __NR_access
+ return INTERNAL_SYSCALL_CALL (access, err, pathname, mode);
+#else
+ return INTERNAL_SYSCALL_CALL (faccessat, err, AT_FDCWD, file, type);
+#endif
+}
--
2.7.4