From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23464 invoked by alias); 8 Nov 2016 17:10:10 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 23447 invoked by uid 89); 8 Nov 2016 17:10:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=2128, 2111, 1816, weak_alias X-HELO: mail-vk0-f50.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=+e4n7zZYEPD4l5BBf5dw/0noQ3HIJpKIsTpoBI0iOQU=; b=AXXy/jONKV1fEGXkvvtYHqk7Ki6Pe45QfXHEXlCzPJq7dJexhrjjmDpEWBlOSdaDq4 6IV6RE12xL7FMTByBleCUpRsGcEXJOHNXFhUXcZEuuxeLQtB2o87XaVw/1VSefVDS27v WnJl2eACJsaDYVXNzO+BxF6cOtjp4fSe+z+OJTQsCGxbPmw8QqYkiYvQN4rBP3gH9q8I XXvhnjy76ZdEjPbIJT/iavaJea48qsDOKL3hGrC4vy8J25LbwFx60SoenPcvpj9P5F8G wrCX3offCRSwNGWF62cf8ni40uz7xgs27OKOJv4wiyJUFmJPRqHhwm8oMImgx49X5o0o 192Q== X-Gm-Message-State: ABUngveONgxoqE41xxQC04Hv9UwgyWFCjVYSSqR1olnKLbb9gJttNcHj+amiKTvhWHOm6FAj X-Received: by 10.31.170.208 with SMTP id t199mr3524783vke.6.1478624997491; Tue, 08 Nov 2016 09:09:57 -0800 (PST) Subject: Re: [PATCH 2/6] New internal function __access_noerrno To: libc-alpha@sourceware.org References: <1477320168-23397-1-git-send-email-siddhesh@sourceware.org> <1477320168-23397-3-git-send-email-siddhesh@sourceware.org> From: Adhemerval Zanella Message-ID: <317e63c8-1681-84b1-e278-c6ecae71e657@linaro.org> Date: Tue, 08 Nov 2016 17:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1477320168-23397-3-git-send-email-siddhesh@sourceware.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-SW-Source: 2016-11/txt/msg00278.txt.bz2 On 24/10/2016 12:42, Siddhesh Poyarekar wrote: > 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. > > * include/unistd.h [IS_IN (rtld) || !defined SHARED]: Declare > __access_noerrno. > * io/Makefile (routines): Add access_noerrno. > * io/access.c (__ACCESS)[!__ACCESS]: Define as __access. > (__access): Rename to __ACCESS. > [!NOERRNO]: Retain default __access logic. > * io/access_noerrno.c: New file. > * sysdeps/mach/hurd/access.c (__ACCESS)[!__ACCESS]: Define as > __access. > (__HURD_FAIL): New macro. > (__access): Rename to __ACCESS. Use __HURD_FAIL instead of > __hurd_fail. > [!NOERRNO]: Set weak alias to access. > * sysdeps/nacl/access.c (__ACCESS)[!__ACCESS]: Define as > __access. > (DO_NACL_CALL): New macro. > (__access): Rename to __ACCESS. Use DO_NACL_CALL instead of > NACL_CALL. > [!NOERRNO]: Set weak alias to access. > * sysdeps/nacl/nacl-interfaces.h (NACL_CALL_NOERRNO): New > macro. > * sysdeps/unix/access_noerrno.c: New file. > * sysdeps/unix/sysv/linux/generic/access.c: Include sysdep.h. > (__ACCESS)[!__ACCESS]: Define as __access. > (__access): Rename to __ACCESS. > [NOERRNO]: Call faccessat syscall without setting errno. > --- > include/unistd.h | 6 +++++ > io/Makefile | 1 + > io/access.c | 10 ++++++++- > io/access_noerrno.c | 21 ++++++++++++++++++ > sysdeps/mach/hurd/access.c | 20 +++++++++++++---- > sysdeps/nacl/access.c | 16 ++++++++++++-- > sysdeps/nacl/nacl-interfaces.h | 4 ++++ > sysdeps/unix/access_noerrno.c | 38 ++++++++++++++++++++++++++++++++ > sysdeps/unix/sysv/linux/generic/access.c | 19 +++++++++++++++- > 9 files changed, 127 insertions(+), 8 deletions(-) > create mode 100644 io/access_noerrno.c > create mode 100644 sysdeps/unix/access_noerrno.c > > 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 > # 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. */ Is this comment correct? Checking the patch I am seeing it builds only for libc and there is no resulting __access_noerrno on ld.so. > diff --git a/sysdeps/unix/access_noerrno.c b/sysdeps/unix/access_noerrno.c > new file mode 100644 > index 0000000..1ff90a2 > --- /dev/null > +++ b/sysdeps/unix/access_noerrno.c > @@ -0,0 +1,38 @@ > +/* Test for access to a file but do not set errno on error. > + Copyright (C) 2016 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + Contributed by Chris Metcalf , 2011. > + > + 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 > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Test for access to FILE. */ > +int > +__access_noerrno (const char *file, int type) > +{ > + INTERNAL_SYSCALL_DECL (err); > + int res; > + res = INTERNAL_SYSCALL (access, err, 2, file, type); > + if (INTERNAL_SYSCALL_ERROR_P (res, err)) > + return INTERNAL_SYSCALL_ERRNO (res, err); > + else > + return 0; > +} I think it would be simpler to just 1. consolidation Linux access implementation and 2. add the '_noerrno' on same file. The 1. would be simpler to just: 1.1. Remove access from sysdeps/unix/syscalls.list 1.2. Move sysdeps/unix/sysv/linux/generic/access.c to sysdeps/unix/sysv/linux/access.c 1.3. And implement access checking for __NR_access and using __NR_facessat if is not defined. Something like: [...] /* Test for access to FILE. */ int __access (const char *file, int type) { #if __NR_access return INLINE_SYSCALL_CALL (access, file, type); #else return INLINE_SYSCALL_CALL (faccessat, AT_FDCWD, file, type); #endif } [...] Then __access_noerro could be just implemented on same file. I think it has the advantage of not splitting the access on multiple files. I think for hurd and nacl it would also result in simplify code. What do you think? > diff --git a/sysdeps/unix/sysv/linux/generic/access.c b/sysdeps/unix/sysv/linux/generic/access.c > index 586aa93..5bafc06 100644 > --- a/sysdeps/unix/sysv/linux/generic/access.c > +++ b/sysdeps/unix/sysv/linux/generic/access.c > @@ -21,11 +21,28 @@ > #include > #include > #include > +#include > + > +#ifndef __ACCESS > +# define __ACCESS __access > +#endif > > /* Test for access to FILE. */ > int > -__access (const char *file, int type) > +__ACCESS (const char *file, int type) > { > +#ifndef NOERRNO > return INLINE_SYSCALL (faccessat, 3, AT_FDCWD, file, type); > +#else > + INTERNAL_SYSCALL_DECL (err); > + int res; > + res = INTERNAL_SYSCALL (faccessat, err, 3, AT_FDCWD, file, type); > + if (INTERNAL_SYSCALL_ERROR_P (res, err)) > + return INTERNAL_SYSCALL_ERRNO (res, err); > + else > + return 0; > +#endif > } > +#ifndef NOERRNO > weak_alias (__access, access) > +#endif >