From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 98188 invoked by alias); 10 Nov 2016 18:31:38 -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 98176 invoked by uid 89); 10 Nov 2016 18:31:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Checked, siddheshsourcewareorg, siddhesh@sourceware.org, U*siddhesh X-HELO: mail-ua0-f178.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=KfgNn9jU3tqyM1R4n/x8UTuCe4l3MOiU9x1gTP/mSv0=; b=B9yavMnwIpLVY3KGd9x9CCO/Ccu+cN2i2sSeLdMCCcRWB9kSWnIOkyuii929jn7NkQ YFo9+Ur/I6c8+Np6EJFM6uvUC7tPi76mmpMWp+wDo8404EWr14p5uPe5GaUCd4Ik3Yhd iGE87gq1gqQzVFaOx4ltsPSqko//MI1IlX29fIw9QyboZmpFV5bKxnKr4WJFqPm5kKji vhkYuoWNSJnDUYDa1L3rzzd+USqra/2HbdcFkQjh0s9uSqBMlhHb4NUANWCwqT4tgbv4 A0m7bIu3Pot3LITAggRM1OEeMYzi7gxPH1Y9nnaR/2wAQY2k+GOAxo1cHgPTIdxGT7K7 Vt6A== X-Gm-Message-State: ABUngvdpbhPb+WG3a122nVpSLOaVTIKGU+YURdBdEYqQkp/Mr1mltf5M3WZQMP4bYq9WZKIx X-Received: by 10.176.0.147 with SMTP id 19mr3833723uaj.20.1478802685501; Thu, 10 Nov 2016 10:31:25 -0800 (PST) Subject: Re: [PATCH 2/2] New internal function __access_noerrno To: Siddhesh Poyarekar , libc-alpha@sourceware.org References: <1478797446-12213-1-git-send-email-adhemerval.zanella@linaro.org> <1478797446-12213-2-git-send-email-adhemerval.zanella@linaro.org> <083c6da8-5c67-acba-f7f0-79fb4253d7d2@gotplt.org> From: Adhemerval Zanella Message-ID: <23ef1354-7ade-70fe-46f0-59392798ef82@linaro.org> Date: Thu, 10 Nov 2016 18:31: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: <083c6da8-5c67-acba-f7f0-79fb4253d7d2@gotplt.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-11/txt/msg00416.txt.bz2 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. > >> >> Checked on x86_64. >> >> Siddhesh Poyarekar >> Adhemerval Zanella >> >> * 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. >> >> [1] https://sourceware.org/ml/libc-alpha/2016-11/msg00399.html >> --- >> ChangeLog | 13 +++++++++++++ >> hurd/hurd.h | 30 ++++++++++++++++++++++++++++++ >> include/unistd.h | 6 ++++++ >> io/access.c | 8 +++++++- >> 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, 113 insertions(+), 7 deletions(-) >> >> diff --git a/hurd/hurd.h b/hurd/hurd.h >> index ec07827..8bcb1ec 100644 >> --- a/hurd/hurd.h >> +++ b/hurd/hurd.h >> @@ -75,6 +75,36 @@ __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; >> + } >> + >> + errno = err; >> + return -1; > > Should not set errno and return it instead to be consistent with what > other architectures do. It might be nicer to have __hurd_fail call > __hurd_fail_noerrno to reduce code duplication. > >> +} >> >> /* 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 >> # 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 >> + >> __END_DECLS >> # endif >> >> diff --git a/io/access.c b/io/access.c >> index 4534704..68b49ca 100644 >> --- a/io/access.c >> +++ b/io/access.c >> @@ -19,6 +19,13 @@ >> #include >> #include >> >> +/* Test for access to FILE without setting errno. */ >> +int >> +__access_noerrno (const char *file, int type) >> +{ >> + return -1; >> +} >> + >> /* Test for access to FILE. */ >> int >> __access (const char *file, int type) >> @@ -33,5 +40,4 @@ __access (const char *file, int type) >> return -1; >> } >> stub_warning (access) >> - >> weak_alias (__access, access) >> 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 >> #include >> >> -/* 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); >> +} >> + >> +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); >> >> 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); >> +} >> weak_alias (__access, access) >> diff --git a/sysdeps/nacl/access.c b/sysdeps/nacl/access.c >> index 95a0fb7..4266d63 100644 >> --- a/sysdeps/nacl/access.c >> +++ b/sysdeps/nacl/access.c >> @@ -19,6 +19,13 @@ >> #include >> #include >> >> +/* Test for access to FILE without setting errno. */ >> +int >> +__access (const char *file, int type) > > __access_noerrno > >> +{ >> + return NACL_CALL_NOERRNO (__nacl_irt_dev_filename.access (file, type), 0); >> +} >> + >> /* 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); }) >> + >> #endif /* nacl-interfaces.h */ >> diff --git a/sysdeps/unix/sysv/linux/access.c b/sysdeps/unix/sysv/linux/access.c >> index cdb7908..004da1b 100644 >> --- a/sysdeps/unix/sysv/linux/access.c >> +++ b/sysdeps/unix/sysv/linux/access.c >> @@ -19,6 +19,21 @@ >> #include >> #include >> >> +int >> +__access_noerro (const char *file, int type) > > Typo, __access_noerrno. > >> +{ >> + 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; >> +} >> + >> /* Test for access to FILE. */ >> int >> __access (const char *file, int type) >>