From patchwork Tue Jul 9 21:35:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dragan Mladjenovic X-Patchwork-Id: 33641 Received: (qmail 94801 invoked by alias); 9 Jul 2019 21:36:04 -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 94792 invoked by uid 89); 9 Jul 2019 21:36:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_WEB, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: NAM03-BY2-obe.outbound.protection.outlook.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CnUyNd1hOH1UeIMrHfxjhGgNeRJuqGhEFQA6N7EnXPim1co6kmLc9NNogdr8nGM47JK5+68nrh9T6HkCDUos3BB5v1DjGFwOD7/atIUDngRCEuii99YOVpPVmmMZKKWwx5l4SnJho30eiBrwo5HW/SIJICQwPBGFvV4CZYnksxVhykflpJN8rjZ7HUzPLwYhnNT4JEbZkkmXjNzYoAlq3vRXhQ7ht3fjmRBheQIqSMBFBKHE2nlc+H37Sf/ZiFSFZivBQR6iElhg2+PC08sJK3HSd3wzTUVNDfKwQV3JVJxY+GEf08mXsy4cZFmDIs7Z2ZBcB/sL0ms/pFBV92kWUA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+tNsbZSPai+Zs6ZUQe0WTbzkgg8jlwdzSYpu4MfiKzQ=; b=lynIJo5Uq6XzxKw3dDCyimWk6oyCg7tWxW+s+yrSgik/bz4QlrH6EyTrCaNMWlMWHZ9dHV/8jlRy3esi53IUih285FMS2AO58t29qTi5gPo+tetYqBTht293+Rh4dkas9EkqhTG1qXI+fiIf4icGUqUrhsh/XaW/C41xXNBwGcRJ1Xw0J9d0XMZiWBletio5XsMyyB+ZLdeIGR5qpL30ghmq0TECQDMqKchrbHBJ5H06THuEBb6l4c9FZpannNXDP/7I6iIrfkLCX+IqxyUOYzpaMlRQBYFw9isWWjortetEKTGDeX1AiUOgdYp6mmagwGn28fqp64uKcLFW8AG7qg== ARC-Authentication-Results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=wavecomp.com;dmarc=pass action=none header.from=wavecomp.com;dkim=pass header.d=wavecomp.com;arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wavesemi.onmicrosoft.com; s=selector1-wavesemi-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+tNsbZSPai+Zs6ZUQe0WTbzkgg8jlwdzSYpu4MfiKzQ=; b=a0BPQnetbxqKcoHM3XzDBTQmRqNcqV9ZqFPozcl/cc0KwMQbtMpZ03thR4ruvSur053vw6P/y+rO2E5lqgL6YRntm2RSogCRcn4okFAyT0yZAZMdPCcY2cL5n+Wf3GE26aetKqkDSL/v4umZIfwaC13fkf9iwEVWGf/i0ZbcK5s= From: Dragan Mladjenovic To: Joseph Myers CC: "libc-alpha@sourceware.org" , Carlos O'Donell , "Maciej W. Rozycki" , Faraz Shahbazker , Florian Weimer Subject: Re: [PATCH 1/3] [ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE. Date: Tue, 9 Jul 2019 21:35:58 +0000 Message-ID: References: <1561672142-5907-1-git-send-email-dmladjenovic@wavecomp.com> <1561672142-5907-2-git-send-email-dmladjenovic@wavecomp.com>, In-Reply-To: authentication-results: spf=none (sender IP is ) smtp.mailfrom=dmladjenovic@wavecomp.com; x-ms-oob-tlc-oobclassifiers: OLM:10000; received-spf: None (protection.outlook.com: wavecomp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: dmladjenovic@wavecomp.com Florian Weimer * > Is the comment really correct? I think the proposed MIPS implementation > is not architecture-specific, but specific to the kernel version. My mistake. I guess the correct term would be machine in glibc nomenclature. Joseph Mayers* >> > +#ifdef USE_DL_EXEC_STACK_OVERRIDE >> > + /* Program requests a non-executable stack, but architecture does >> > + not support it. */ >> > + if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0)) >> > + { >> > + errstring = N_("cannot override stack memory protections"); >> > + goto call_lose_errno; >> > + } >> > +#endif > This sort of #ifdef is not proper glibc style. You should have a default > trivial (inline?) definition of _dl_exec_stack_override and then have MIPS > override the file with that function definition (without duplicating any > architecture-independent code in the process). If you have a default > inline function definition, that means all this code gets checked for > syntax when building for any architecture, not just for MIPS. > Is patch below more suitable? We would have a common _dl_exec_stack_override that would (preferably) not be overridden by the machine support, but instead one can define DL_EXEC_STACK_OVERRIDE or maybe DL_EXEC_STACK_OVERRIDE_P to control the conditions under which the stack "override" happens. I realized that I don't actually need to duplicate the __stack_prot unprotect/protect code from elf/dl-load.c, so I moved the dynamic linking case into dl-main. This way the version check is done at most once. Static linking case is still done as part of _dl_non_dynamic_init. If we ever gain support for IFUNC on MIPS we would probably need to move this somewhere before running IFUNC revolvers. What are your thoughts on this? diff --git a/elf/dl-exec-stack-override.h b/elf/dl-exec-stack-override.h new file mode 100644 index 0000000..10401a8 --- /dev/null +++ b/elf/dl-exec-stack-override.h @@ -0,0 +1,36 @@ +/* Make stack executable if the machine requires it. Generic version. ... + +#include + +extern int __stack_prot attribute_relro attribute_hidden; + +static __always_inline void +_dl_exec_stack_override (void) +{ + if (__glibc_unlikely ((GL(dl_stack_flags) & PF_X) == 0 + && DL_EXEC_STACK_OVERRIDE)) + { + __stack_prot |= PROT_READ|PROT_WRITE|PROT_EXEC; + + void *stack_end = __libc_stack_end; + int err = _dl_make_stack_executable (&stack_end); + if (__glibc_unlikely (err)) + _dl_fatal_printf ("cannot enable executable stack as machine requires\n"); + } +} diff --git a/elf/dl-support.c b/elf/dl-support.c index 0a8b636..923aa4c 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -375,6 +376,8 @@ _dl_non_dynamic_init (void) _dl_stack_flags = _dl_phdr[i].p_flags; break; } + + _dl_exec_stack_override (); } #ifdef DL_SYSINFO_IMPLEMENTATION diff --git a/elf/rtld.c b/elf/rtld.c index c9490ff..f3e00f9 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -1542,6 +1543,8 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]); DL_SYSDEP_OSCHECK (_dl_fatal_printf); #endif + _dl_exec_stack_override (); + /* Initialize the data structures for the search paths for shared objects. */ _dl_init_paths (library_path); diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index b1fc5c3..70e96c0 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -119,6 +119,10 @@ dl_symbol_visibility_binds_local_p (const ElfW(Sym) *sym) # define DL_STATIC_INIT(map) #endif +#ifndef DL_EXEC_STACK_OVERRIDE +# define DL_EXEC_STACK_OVERRIDE false +#endif + /* Reloc type classes as returned by elf_machine_type_class(). ELF_RTYPE_CLASS_PLT means this reloc should not be satisfied by some PLT symbol, ELF_RTYPE_CLASS_COPY means this reloc should not be