This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/2] New internal function __access_noerrno
On 11/16/2016 08:44 AM, Adhemerval Zanella wrote:
>
>
> On 16/11/2016 11:27, Adhemerval Zanella wrote:
>>
>>
>> On 10/11/2016 16:31, Adhemerval Zanella wrote:
>>>
>>>
>>> On 10/11/2016 15:53, Siddhesh Poyarekar wrote:
>>>> On Thursday 10 November 2016 10:34 PM, Adhemerval Zanella wrote:
>>>>> This ia follow up patch for tunables requirement [1]. It Implement
>>>>> an internal version of __access called __access_noerrno that
>>>>> avoids setting errno. This is useful to check accessibility of files
>>>>> very early on in process startup i.e. before TLS setup. This allows
>>>>> tunables to replace MALLOC_CHECK_ safely (i.e. check existence of
>>>>> /etc/suid-debug to enable/disable MALLOC_CHECK) and at the same time
>>>>> initialize very early so that it can override IFUNCs.
>>>>
>>>> I think someone else should also review and ack this one, but I'll do a
>>>> review round anyway.
>>>
>>> Thanks, I fixes all my mistakes locally. It would be good to have a
>>> ack for nacl/hurd before pushing it.
>>
>> I tried both hurd [1] and nacl [2] environments to check the build but
>> without success. Hurd VM does not boot with a recent qemu (2.7.50) and
>> NaCL toolchain seems stuck in a ancient gcc version (4.4.7). Since I
>> this patch won't break any functionality (since it only adds a new
>> symbol), I see it should be safe to push.
>>
>> [1] https://people.debian.org/~sthibault/hurd-i386/README
>> [2] https://developer.chrome.com/native-client/sdk/download
Agreed, if we can't get building solutions for nacl and hurd then we can't
test them. I've discussed this before without contributors and it's a serious
issue for hurd that we really need to fix, but that is not your responsibility.
> Updated version below:
>
> * hurd/hurd.h (__hurd_fail_noerrno): New function.
> * include/unistd.h [IS_IN (rtld) || !defined SHARED]: Declare
> __access_noerrno.
> * io/access.c (__access_noerrno): New function.
> * sysdeps/mach/hurd/access.c (hurd_fail_seterrno): New function.
> (hurd_fail_seterrno): Likewise.
> (access_common): Likewise.
> (__access_noerrno): Likewise.
> * sysdeps/nacl/access.c (__access_noerrno): Likewise.
> * sysdeps/unix/sysv/linux/access.c (__access_noerrno): Likewise.
> * sysdeps/nacl/nacl-interfaces.h (NACL_CALL_NOERRNO): New
> macro.
This patch looks good to me.
> ---
> ChangeLog | 15 +++++++++++++++
> hurd/hurd.h | 29 +++++++++++++++++++++++++++++
> include/unistd.h | 6 ++++++
> io/access.c | 7 +++++++
> sysdeps/mach/hurd/access.c | 37 +++++++++++++++++++++++++++++++------
> sysdeps/nacl/access.c | 7 +++++++
> sysdeps/nacl/nacl-interfaces.h | 4 ++++
> sysdeps/unix/sysv/linux/access.c | 15 +++++++++++++++
> 8 files changed, 114 insertions(+), 6 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index dfa48e4..b2e5b68 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,18 @@
> +2016-11-16 Adhemerval Zanella <adhemerval.zanella@linaro.org>
> +
> + * hurd/hurd.h (__hurd_fail_noerrno): New function.
> + * include/unistd.h [IS_IN (rtld) || !defined SHARED]: Declare
> + __access_noerrno.
> + * io/access.c (__access_noerrno): New function.
> + * sysdeps/mach/hurd/access.c (hurd_fail_seterrno): New function.
> + (hurd_fail_seterrno): Likewise.
> + (access_common): Likewise.
> + (__access_noerrno): Likewise.
> + * sysdeps/nacl/access.c (__access_noerrno): Likewise.
> + * sysdeps/unix/sysv/linux/access.c (__access_noerrno): Likewise.
> + * sysdeps/nacl/nacl-interfaces.h (NACL_CALL_NOERRNO): New
> + macro.
> +
> 2016-11-16 Joseph Myers <joseph@codesourcery.com>
>
> * sysdeps/unix/sysv/linux/sh/sh4/register-dump.h (register_dump):
> diff --git a/hurd/hurd.h b/hurd/hurd.h
> index ec07827..c089d4c 100644
> --- a/hurd/hurd.h
> +++ b/hurd/hurd.h
> @@ -75,6 +75,35 @@ __hurd_fail (error_t err)
> errno = err;
> return -1;
> }
> +
> +_HURD_H_EXTERN_INLINE int
> +__hurd_fail_noerrno (error_t err)
> +{
> + switch (err)
> + {
> + case EMACH_SEND_INVALID_DEST:
> + case EMIG_SERVER_DIED:
> + /* The server has disappeared! */
> + err = EIEIO;
> + break;
> +
> + case KERN_NO_SPACE:
> + err = ENOMEM;
> + break;
> +
> + case KERN_INVALID_ARGUMENT:
> + err = EINVAL;
> + break;
> +
> + case 0:
> + return 0;
> +
> + default:
> + break;
> + }
> +
> + return -1;
> +}
OK.
>
> /* Basic ports and info, initialized by startup. */
>
> diff --git a/include/unistd.h b/include/unistd.h
> index d2802b2..6144f41 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -181,6 +181,12 @@ 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. */
> +extern __typeof (__access) __access_noerrno attribute_hidden;
> +# endif
OK.
> +
> __END_DECLS
> # endif
>
> diff --git a/io/access.c b/io/access.c
> index 4534704..859cf75 100644
> --- a/io/access.c
> +++ b/io/access.c
> @@ -19,6 +19,13 @@
> #include <stddef.h>
> #include <unistd.h>
>
> +/* Test for access to FILE without setting errno. */
> +int
> +__access_noerrno (const char *file, int type)
> +{
> + return -1;
> +}
OK.
> +
> /* Test for access to FILE. */
> int
> __access (const char *file, int type)
> diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c
> index c308340..620acea 100644
> --- a/sysdeps/mach/hurd/access.c
> +++ b/sysdeps/mach/hurd/access.c
> @@ -22,9 +22,20 @@
> #include <hurd/lookup.h>
> #include <fcntl.h>
>
> -/* Test for access to FILE by our real user and group IDs. */
> -int
> -__access (const char *file, int type)
> +static int
> +hurd_fail_seterrno (error_t err)
> +{
> + return __hurd_fail (err);
> +}
> +
> +static int
> +hurd_fail_noerrno (error_t err)
> +{
> + return __hurd_fail_noerrno (err);
> +}
OK.
> +
> +static int
> +access_common (const char *file, int type, int (*errfunc) (error_t))
> {
> error_t err;
> file_t rcrdir, rcwdir, io;
> @@ -120,13 +131,13 @@ __access (const char *file, int type)
> if (rcwdir != MACH_PORT_NULL)
> __mach_port_deallocate (__mach_task_self (), rcwdir);
> if (err)
> - return __hurd_fail (err);
> + return errfunc (err);
>
> /* Find out what types of access we are allowed to this file. */
> err = __file_check_access (io, &allowed);
> __mach_port_deallocate (__mach_task_self (), io);
> if (err)
> - return __hurd_fail (err);
> + return errfunc (err);
>
> flags = 0;
> if (type & R_OK)
> @@ -138,9 +149,23 @@ __access (const char *file, int type)
>
> if (flags & ~allowed)
> /* We are not allowed all the requested types of access. */
> - return __hurd_fail (EACCES);
> + return errfunc (EACESS);
>
OK.
> return 0;
> }
>
> +/* Test for access to FILE by our real user and group IDs without setting
> + errno. */
> +int
> +__access_noerrno (const char *file, int type)
> +{
> + return access_common (file, type, hurd_fail_noerrno);
> +}
> +
> +/* Test for access to FILE by our real user and group IDs. */
> +int
> +__access (const char *file, int type)
> +{
> + return access_common (file, type, hurd_fail);
> +}
OK.
> weak_alias (__access, access)
> diff --git a/sysdeps/nacl/access.c b/sysdeps/nacl/access.c
> index 95a0fb7..e07d558 100644
> --- a/sysdeps/nacl/access.c
> +++ b/sysdeps/nacl/access.c
> @@ -19,6 +19,13 @@
> #include <unistd.h>
> #include <nacl-interfaces.h>
>
> +/* Test for access to FILE without setting errno. */
> +int
> +__access_noerrno (const char *file, int type)
> +{
> + return NACL_CALL_NOERRNO (__nacl_irt_dev_filename.access (file, type), 0);
> +}
> +
OK.
> /* Test for access to FILE. */
> int
> __access (const char *file, int type)
> diff --git a/sysdeps/nacl/nacl-interfaces.h b/sysdeps/nacl/nacl-interfaces.h
> index b7b45bb..edd3217 100644
> --- a/sysdeps/nacl/nacl-interfaces.h
> +++ b/sysdeps/nacl/nacl-interfaces.h
> @@ -113,4 +113,8 @@ __nacl_fail (int err)
> #define NACL_CALL(err, val) \
> ({ int _err = (err); _err ? __nacl_fail (_err) : (val); })
>
> +/* Same as NACL_CALL but without setting errno. */
> +#define NACL_CALL_NOERRNO(err, val) \
> + ({ int _err = (err); _err ? _err : (val); })
> +
OK.
> #endif /* nacl-interfaces.h */
> diff --git a/sysdeps/unix/sysv/linux/access.c b/sysdeps/unix/sysv/linux/access.c
> index 8f003df..adefbca 100644
> --- a/sysdeps/unix/sysv/linux/access.c
> +++ b/sysdeps/unix/sysv/linux/access.c
> @@ -21,6 +21,21 @@
> #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;
> +}
OK.
> +
> +int
> __access (const char *file, int type)
> {
> #ifdef __NR_access
>
--
Cheers,
Carlos.