From patchwork Thu Jul 20 12:46:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 791585 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-82295-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="VLZu0d52"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xCtty17xjz9s5L for ; Thu, 20 Jul 2017 22:46:33 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=FLs554jBzt5gH9FK MPmMe7vFovaqjTWzSvGp6lJcnuV47ApMzDaNc1geisBxP80IL/wKy5m8ilPW3G/5 F5CUPHUQ02Kuju+15TqZLiFNKSuLF2EJ7pjqj37ZhTn/1W6PqbiVb4vzUwy0GPQX Bb8JlKmmmXV8rcUzFpQCQgMpVi0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=yFtt6uHdzX4ePwjhGITinh wITuU=; b=VLZu0d52VfXmYWjVydWZ4kFXem5kkkbPLD4yxS03+qzsVYS14l57sm S6+AR1E0BjisW56WHD9Yvw6wdr+QJI1sYVZCclSTCETRRRADDskxzn3xD0ENcGcm +MxMnKUnY4y9fZVnARUlQYd1ZevbBe7sDYjGxiwaimSdAO3XKakMQ= Received: (qmail 96938 invoked by alias); 20 Jul 2017 12:46:27 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 96929 invoked by uid 89); 20 Jul 2017 12:46:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-27.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f178.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=pY8XuIf/n9K7aOubtEYo2v63dAB1eP4RFW6tksmEdiQ=; b=gmlyNfhdU2pKtCUX97vflDuDJn0TNM0o+4vReZjicUuW10fP7aItcS4jp1DPJmqUpi Cv+LARbZL9nyEQbDWggs+939K8a4M5LJMx3I/lm4a4hBAgNf7QXBG3AqxA1HWWl3sJxY AfEkUlUm1vdWbpnvMFJwZob99tuPnR4rt6CkUK1v7p5gIr5nmhgdmX0EhE5JNN26XWlw oTyQDIxfJKzw6T2h6dZByGDoTE5PNMtF5ATLqkl7t1b0uKNC5QRNtvqVxBgG2FCDdUeF eGqyoTz8L8B5svZr4B4l8wRuiqmVIs1Zvx4xQ5XJpo/lne/4u6sDR9IaAMZDFufHvnyv 2t/A== X-Gm-Message-State: AIVw112+dk65EzeBEEY849dRpLY8PMM6EkKtidvZCWJl6jBhwDCkCxwx vz75v4HdbeOhDKkAmGKNOg== X-Received: by 10.55.55.79 with SMTP id e76mr4732812qka.13.1500554773876; Thu, 20 Jul 2017 05:46:13 -0700 (PDT) Subject: Re: [PATCH] Don't all __access_noerrno with stack protector from __tunables_init [BZ #21744] To: "H.J. Lu" Cc: Florian Weimer , GNU C Library References: <20170716191924.GA7226@gmail.com> <7643d65d-839e-a85d-8a4f-30eada595887@linaro.org> <874luag5n4.fsf@mid.deneb.enyo.de> <6dc48782-60c3-cead-08c0-7e018e7eb9ef@linaro.org> <80b843b8-6e3c-ccbe-5a6f-b37e18dbdbae@linaro.org> <209c41dd-d940-0625-f29b-0001a9b7019a@linaro.org> From: Adhemerval Zanella Message-ID: <98fc5143-e689-9e5b-d005-c9f22eaecc68@linaro.org> Date: Thu, 20 Jul 2017 09:46:10 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: On 19/07/2017 22:11, H.J. Lu wrote: > On Wed, Jul 19, 2017 at 5:56 PM, Adhemerval Zanella > wrote: >> >> >> On 19/07/2017 18:53, H.J. Lu wrote: >>> On Tue, Jul 18, 2017 at 6:52 AM, Adhemerval Zanella >>> 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 >>>> Adhemerval Zanella >>> >>> 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 >>>> + >>>> #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 >>>> # 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 >>>> + . */ >>>> + >>>> +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 >>>> >>>> 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 >>>> + . */ >>>> + >>>> +/* 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 + #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 # 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 + . */ + +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 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 + . */ + +/* 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 +}