From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73920 invoked by alias); 2 Dec 2019 13:57:57 -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 73908 invoked by uid 89); 2 Dec 2019 13:57:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*r:ip*192.168.1.4, HX-Spam-Relays-External:!192.168.1.4!, H*RU:!192.168.1.4! X-HELO: mail-qk1-f196.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=oppc92dpsbno3FAtHhpClPPhuh741Kgo1KQYqPHi/WQ=; b=YqYnIXpH44fU6xWiEhHtTmpqPiGDcUc34Cx2Et4MfVXISTu2IVzlO6nL8s5M2+urO6 j+9HSGdl6tlIG8oIPgkU7Kw/NNcbxXLP165Twhwzl+wKi7xWV1AAKC0yHv0xMWfb34xH 0dEpO5255e9DYkL/V5gDFKWKLmJ6Fp22UPxGPQprJhX6EtNBI9MOOBdBHiQIFpOGCOpw hkCLd+oYibYrdgvNRtH6XahMUIlGr62Y5Spv8JwbndwXnNhfFYrXHjqG/jXPjSVggGx0 Folp+/RDK0xajiA9n8qoQOlr1QGTMrJmldKc6YqOhnvxtKJnFnBq6/Zv8CG7vPUmeIUl ONVA== Return-Path: To: Florian Weimer Cc: libc-alpha@sourceware.org References: <20191129210327.26434-1-adhemerval.zanella@linaro.org> <20191129210327.26434-5-adhemerval.zanella@linaro.org> <874kykfjyy.fsf@mid.deneb.enyo.de> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH 5/7] elf: Enable relro for static build Message-ID: <104ece90-2d18-d655-dafd-1084e32a9c4f@linaro.org> Date: Mon, 02 Dec 2019 13:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <874kykfjyy.fsf@mid.deneb.enyo.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-12/txt/msg00022.txt.bz2 On 01/12/2019 06:55, Florian Weimer wrote: > * Adhemerval Zanella: > >> The code is similar to the one at rtld.c, where its check for the >> PT_GNU_RELRO header values from program headers and call >> _dl_protected_relro with the updated l_relro_{addr,size} values. > > This is not the actual code that does RELRO in most cases, it's only > used with prelink. _dl_relocate_object is what is used. Ack, I changed the commit message to: The code is similar to the one at elf/dl-reloc.c, where it checks for the l_relro_size from the link_map (obtained from PT_GNU_RELRO header from program headers) and calls_dl_protected_relro. > >> diff --git a/elf/dl-support.c b/elf/dl-support.c >> index 5526d5ee6e..bdb5c2ae91 100644 >> --- a/elf/dl-support.c >> +++ b/elf/dl-support.c >> @@ -367,14 +367,24 @@ _dl_non_dynamic_init (void) >> if (_dl_platform != NULL) >> _dl_platformlen = strlen (_dl_platform); >> >> - /* Scan for a program header telling us the stack is nonexecutable. */ >> if (_dl_phdr != NULL) >> - for (uint_fast16_t i = 0; i < _dl_phnum; ++i) >> - if (_dl_phdr[i].p_type == PT_GNU_STACK) >> + for (const ElfW(Phdr) *ph = _dl_phdr; ph < &_dl_phdr[_dl_phnum]; ++ph) >> + switch (ph->p_type) >> { >> - _dl_stack_flags = _dl_phdr[i].p_flags; >> + /* Check if the stack is nonexecutable. */ >> + case PT_GNU_STACK: >> + _dl_stack_flags = ph->p_flags; >> + break; >> + >> + case PT_GNU_RELRO: >> + _dl_main_map.l_relro_addr = ph->p_vaddr; >> + _dl_main_map.l_relro_size = ph->p_memsz; >> break; >> } >> + >> + /* Setup relro on the binary itself. */ >> + if (_dl_main_map.l_relro_size) >> + _dl_protect_relro (&_dl_main_map); > > Please use an explicit comparison with != 0. Ack. > > I have a test case for this which I can post. Sure, I can attach on the patch itself. > Somewhat bizarrely, > full RELRO for statically linked binaries requires linking with -z now. > My understanding it is arch-specific and also depends on how bintuils was build. For instance, with my system ld (GNU ld (GNU Binutils for Ubuntu) 2.30) seemed to be built with DEFAULT_LD_Z_RELRO (set by --enable-relro) which sets relro by default. With this binutils I could only disable relro by explicit add norelro, the -z {lazy,now} did not change the GNU_RELRO header creation. Also the code in ld/emultempl/elf.em does seem to select different linker scripts for both link_info.relro and (link_info.flags & DF_BIND_NOW), however it does no have a special case for link_info.relro and !(link_info.flags & DF_BIND_NOW). I don't see how -relro is requiring -z now on ld code.