From patchwork Mon Jul 17 20:32:31 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: 21653 Received: (qmail 65982 invoked by alias); 17 Jul 2017 20:32:39 -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 65925 invoked by uid 89); 17 Jul 2017 20:32:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qk0-f170.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:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Cx17VaoHaNEBIsnjbZ2QCtB8Lw+xiuYBUfVJuyES7tg=; b=Uk4kztjgD9KgzPqHn0QVFYpthcBDRa+GrP+V7qedVhb/x1ceyeLl6bZruc4TXdl/Jb OmEJ+K8mrvLPW5+TsjjK8GiuqV16LjhYGtcZVukbVzXyyO9Dz1lVEEGEHXy5qi5jMxFF p7w7rjkqvEz+WEj4h1Eh+Kj4aV0HfMmk+ZwrTh/IjD6OTgS7SN1rlqoOhciDy35NV5l4 X4DXWea6P81/ExgerrsQDwbCj4bRnyRD9WFEKvW+uW/yroRT+g9HckEKlkErfFILV4aG O6dOwYr9WxwPoWgbF1u7UEF5al4h2dWOtWgPSkfPdRau7Z3KD+xVm9lGZwbo5zSjDNt9 SUcg== X-Gm-Message-State: AIVw112GsAdBVW5qRGIij3Ditxnk2H4UzZDHSDB2sCnMJ6RlQ15teumH sO8SrTYHcoxssr4qiELsrA== X-Received: by 10.55.64.73 with SMTP id n70mr27925489qka.35.1500323554541; Mon, 17 Jul 2017 13:32:34 -0700 (PDT) Subject: Re: [PATCH] Don't all __access_noerrno with stack protector from __tunables_init [BZ #21744] To: libc-alpha@sourceware.org References: <20170716191924.GA7226@gmail.com> From: Adhemerval Zanella Message-ID: <7643d65d-839e-a85d-8a4f-30eada595887@linaro.org> Date: Mon, 17 Jul 2017 17:32:31 -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: <20170716191924.GA7226@gmail.com> On 16/07/2017 16:19, H.J. Lu wrote: > maybe_enable_malloc_check, which is called by __tunables_init, call > __access_noerrno. It isn't problem when maybe_enable_malloc_check 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 maybe_enable_malloc_check to call _dl_access_noerrno > instead. For ld.so or glibc built without stack protector, it is defined > to __access_noerrno. Otherwise a special version of __access_noerrno > without stack protector is used by maybe_enable_malloc_check in libc.a. > > Tested on x86-64 with and without --enable-stack-protector=all. > I think a much more simpler solution would be just to inline the access call on 'maybe_enable_malloc_check': We can cleanup the non required access_noerro later. > OK for master? > > H.J. > --- > [BZ #21744] > * config.h.in (HAVE_STACK_PROTECTOR): New. > * configure.ac (HAVE_STACK_PROTECTOR): AC_DEFINE if configured > with stack protector. > * configure: Regenerated. > * elf/dl-support.c [HAVE_TUNABLES != 0]: Include if > __access_noerrno is defined. > * elf/dl-tunables.c (maybe_enable_malloc_check): Replace > __access_noerrno with _dl_access_noerrno. > * elf/dl-tunables.h [SHARED != 0 || HAVE_STACK_PROTECTOR == 0] > (_dl_access_noerrno): New macro. > [SHARED == 0 && HAVE_STACK_PROTECTOR != 0] (__access_noerrno): > New macro. > [SHARED == 0 && HAVE_STACK_PROTECTOR != 0] (_dl_access_noerrno): > New prototype. > * io/access.c (__access): Don't define if __access_noerrno is > defined. > * sysdeps/mach/hurd/access.c (__access): Likewise. > * sysdeps/unix/sysv/linux/access.c (__access): Likewise. > --- > config.h.in | 4 ++++ > configure | 6 ++++++ > configure.ac | 3 +++ > elf/dl-support.c | 8 +++++++- > elf/dl-tunables.c | 3 ++- > elf/dl-tunables.h | 14 ++++++++++++++ > io/access.c | 2 ++ > sysdeps/mach/hurd/access.c | 2 ++ > sysdeps/unix/sysv/linux/access.c | 2 ++ > 9 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/config.h.in b/config.h.in > index 22418576a0..01b1a5374b 100644 > --- a/config.h.in > +++ b/config.h.in > @@ -250,4 +250,8 @@ > in i386 6 argument syscall issue). */ > #define CAN_USE_REGISTER_ASM_EBP 0 > > +/* Build glibc with -fstack-protector, -fstack-protector-all, or > + -fstack-protector-strong. */ > +#define HAVE_STACK_PROTECTOR 0 > + > #endif > diff --git a/configure b/configure > index d8e1c50e11..5cc8b2f979 100755 > --- a/configure > +++ b/configure > @@ -4051,14 +4051,20 @@ if test "$enable_stack_protector" = yes && test "$libc_cv_ssp" = yes; then > stack_protector="-fstack-protector" > $as_echo "#define STACK_PROTECTOR_LEVEL 1" >>confdefs.h > > + $as_echo "#define HAVE_STACK_PROTECTOR 1" >>confdefs.h > + > elif test "$enable_stack_protector" = all && test "$libc_cv_ssp_all" = yes; then > stack_protector="-fstack-protector-all" > $as_echo "#define STACK_PROTECTOR_LEVEL 2" >>confdefs.h > > + $as_echo "#define HAVE_STACK_PROTECTOR 1" >>confdefs.h > + > elif test "$enable_stack_protector" = strong && test "$libc_cv_ssp_strong" = yes; then > stack_protector="-fstack-protector-strong" > $as_echo "#define STACK_PROTECTOR_LEVEL 3" >>confdefs.h > > + $as_echo "#define HAVE_STACK_PROTECTOR 1" >>confdefs.h > + > else > stack_protector="-fno-stack-protector" > $as_echo "#define STACK_PROTECTOR_LEVEL 0" >>confdefs.h > diff --git a/configure.ac b/configure.ac > index 77456aa8d9..05a0182792 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -687,12 +687,15 @@ fi > if test "$enable_stack_protector" = yes && test "$libc_cv_ssp" = yes; then > stack_protector="-fstack-protector" > AC_DEFINE(STACK_PROTECTOR_LEVEL, 1) > + AC_DEFINE(HAVE_STACK_PROTECTOR) > elif test "$enable_stack_protector" = all && test "$libc_cv_ssp_all" = yes; then > stack_protector="-fstack-protector-all" > AC_DEFINE(STACK_PROTECTOR_LEVEL, 2) > + AC_DEFINE(HAVE_STACK_PROTECTOR) > elif test "$enable_stack_protector" = strong && test "$libc_cv_ssp_strong" = yes; then > stack_protector="-fstack-protector-strong" > AC_DEFINE(STACK_PROTECTOR_LEVEL, 3) > + AC_DEFINE(HAVE_STACK_PROTECTOR) > else > stack_protector="-fno-stack-protector" > AC_DEFINE(STACK_PROTECTOR_LEVEL, 0) > diff --git a/elf/dl-support.c b/elf/dl-support.c > index c22be854f4..3228cf4020 100644 > --- a/elf/dl-support.c > +++ b/elf/dl-support.c > @@ -164,7 +164,13 @@ uint64_t _dl_hwcap2 __attribute__ ((nocommon)); > /* The value of the FPU control word the kernel will preset in hardware. */ > fpu_control_t _dl_fpu_control = _FPU_DEFAULT; > > -#if !HAVE_TUNABLES > +#if HAVE_TUNABLES > +# ifdef __access_noerrno > +/* If __access_noerrno is defined, include to define the > + special version of __access_noerrno without stack protector. */ > +# include > +# endif > +#else > /* This is not initialized to HWCAP_IMPORTANT, matching the definition > of _dl_important_hwcaps, below, where no hwcap strings are ever > used. This mask is still used to mediate the lookups in the cache > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 44c160cac5..3fef367d72 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -281,7 +281,8 @@ __always_inline > maybe_enable_malloc_check (void) > { > tunable_id_t id = TUNABLE_ENUM_NAME (glibc, malloc, check); > - if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) == 0) > + if (__libc_enable_secure > + && _dl_access_noerrno ("/etc/suid-debug", F_OK) == 0) > tunable_list[id].security_level = TUNABLE_SECLEVEL_NONE; > } > > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > index c92882acba..62d5ef841f 100644 > --- a/elf/dl-tunables.h > +++ b/elf/dl-tunables.h > @@ -30,6 +30,20 @@ __tunables_init (char **unused __attribute__ ((unused))) > } > #else > > +# if defined SHARED || !HAVE_STACK_PROTECTOR > +/* maybe_enable_malloc_check in ld.so uses the special version of > + __access_noerrno without stack protector. When glibc is built > + without stack protector, maybe_enable_malloc_check in libc.a uses > + the regular version of __access_noerrno, which is compiled without > + stack protector. */ > +# define _dl_access_noerrno __access_noerrno > +# else > +/* When glibc is built with stack protector, maybe_enable_malloc_check > + in libc.a must use the special version of __access_noerrno without > + stack protector. */ > +# define __access_noerrno _dl_access_noerrno > +extern int _dl_access_noerrno (const char *, int); > +# endif > # include > # include "dl-tunable-types.h" > > diff --git a/io/access.c b/io/access.c > index 72eba36e3f..d27e789923 100644 > --- a/io/access.c > +++ b/io/access.c > @@ -26,6 +26,7 @@ __access_noerrno (const char *file, int type) > return -1; > } > > +#ifndef __access_noerrno > /* Test for access to FILE. */ > int > __access (const char *file, int type) > @@ -42,3 +43,4 @@ __access (const char *file, int type) > stub_warning (access) > > weak_alias (__access, access) > +#endif > diff --git a/sysdeps/mach/hurd/access.c b/sysdeps/mach/hurd/access.c > index 185e000c9d..329a9fc70a 100644 > --- a/sysdeps/mach/hurd/access.c > +++ b/sysdeps/mach/hurd/access.c > @@ -164,6 +164,7 @@ __access_noerrno (const char *file, int type) > return access_common (file, type, hurd_fail_noerrno); > } > > +#ifndef __access_noerrno > /* Test for access to FILE by our real user and group IDs. */ > int > __access (const char *file, int type) > @@ -171,3 +172,4 @@ __access (const char *file, int type) > return access_common (file, type, hurd_fail_seterrno); > } > weak_alias (__access, access) > +#endif > diff --git a/sysdeps/unix/sysv/linux/access.c b/sysdeps/unix/sysv/linux/access.c > index 67e69bd163..f0e7dd2e83 100644 > --- a/sysdeps/unix/sysv/linux/access.c > +++ b/sysdeps/unix/sysv/linux/access.c > @@ -35,6 +35,7 @@ __access_noerrno (const char *file, int type) > return 0; > } > > +#ifndef __access_noerrno > int > __access (const char *file, int type) > { > @@ -45,3 +46,4 @@ __access (const char *file, int type) > #endif > } > weak_alias (__access, access) > +#endif > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 44c160c..e6db258 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -281,7 +281,8 @@ __always_inline maybe_enable_malloc_check (void) { tunable_id_t id = TUNABLE_ENUM_NAME (glibc, malloc, check); - if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) == 0) + if (__libc_enable_secure + && INTERNAL_SYSCALL_CALL (access, "/etc/suid-debug", F_OK) == 0) tunable_list[id].security_level = TUNABLE_SECLEVEL_NONE; }