Don't all __access_noerrno with stack protector from __tunables_init [BZ #21744]

Message ID cf2078d9-665e-c77d-36ad-29285719ff86@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella July 17, 2017, 9:12 p.m. UTC
  On 17/07/2017 17:41, Adhemerval Zanella wrote:
> 
> 
> On 17/07/2017 17:39, Florian Weimer wrote:
>> * H. J. Lu:
>>
>>> I don't think it works for Hurd.
> 
> Right, but I still think it would be better to make the access call 
> platform specific and call __access_noerrno for Hurd and issue
> an inline syscall for Linux.
> 
>>
>> Yes, generic code cannot use INTERNAL_SYSCALL_CALL.  And it would have
>> to be INLINE_SYSCALL_CALL to prevent the errno access, I think.
>>
> 
> INLINE_SYCALL_CALL is the one that sets errno in fact.
> 

What about (this still require some comments why __access_noerrno is
inline, proper CL, cleanup the Linux implementation, etc.):
  

Comments

H.J. Lu July 18, 2017, 1:44 p.m. UTC | #1
On Mon, Jul 17, 2017 at 2:12 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 17/07/2017 17:41, Adhemerval Zanella wrote:
>>
>>
>> On 17/07/2017 17:39, Florian Weimer wrote:
>>> * H. J. Lu:
>>>
>>>> I don't think it works for Hurd.
>>
>> Right, but I still think it would be better to make the access call
>> platform specific and call __access_noerrno for Hurd and issue
>> an inline syscall for Linux.
>>
>>>
>>> Yes, generic code cannot use INTERNAL_SYSCALL_CALL.  And it would have
>>> to be INLINE_SYSCALL_CALL to prevent the errno access, I think.
>>>
>>
>> INLINE_SYCALL_CALL is the one that sets errno in fact.
>>
>
> What about (this still require some comments why __access_noerrno is
> inline, proper CL, cleanup the Linux implementation, etc.):
>
> 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/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h
> new file mode 100644
> index 0000000..9ac9604
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/not-errno.h
> @@ -0,0 +1,23 @@
> +/* 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/>.  */
> +
> +static inline int
> +__access_noerrno (const char *pathname, int mode)
> +{
> +  return INTERNAL_SYSCALL_CALL (access, pathname, mode);
> +}

This should work for Linux.

Thanks.
  

Patch

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/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h
new file mode 100644
index 0000000..9ac9604
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/not-errno.h
@@ -0,0 +1,23 @@ 
+/* 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/>.  */
+
+static inline int
+__access_noerrno (const char *pathname, int mode)
+{
+  return INTERNAL_SYSCALL_CALL (access, pathname, mode);
+}