From patchwork Fri Dec 9 03:38:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Modra X-Patchwork-Id: 87378 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp108428qgi; Thu, 8 Dec 2016 19:40:09 -0800 (PST) X-Received: by 10.99.143.90 with SMTP id r26mr138821489pgn.164.1481254809875; Thu, 08 Dec 2016 19:40:09 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id m24si31582594pfg.258.2016.12.08.19.40.09 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Dec 2016 19:40:09 -0800 (PST) Received-SPF: pass (google.com: domain of binutils-return-94831-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of binutils-return-94831-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=binutils-return-94831-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; q=dns; s=default; b=xAWd P/YjKu5syBBVmcCDcrUP6s22diopaGDCKxL3dw89QtfEA6g+jSNQK7TEdTkuHxvR eSflXZCZTuM29LUQK9wuV5xNpR5yGdu4bUQzY0S7H2Z89o03Nh5da8ckvqHz59Ew Gv2lKiIM2HmA/3HXBKf4oaI31bFs0zH9PpeKaTM= 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:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; s=default; bh=5JrcC6hrRN /j8jOMLW75EYeZm2I=; b=Dc3LE6J/Yj5xtEbrz9kGixmQKCRcO9BaPanppkWpsM Ar7hT1lmV7mU02ckE52mbQb0cXrEGbuZla1RnSOnwu4NKC3u79egYVcQPbiOVnXH rw+GD956s4CPpzNOKiKjK2RkEJ9TpjH83U3jGpvIFhQrauSLkZxMbU/cB7VQ4Gb0 U= Received: (qmail 42736 invoked by alias); 9 Dec 2016 03:39:53 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Delivered-To: mailing list binutils@sourceware.org Received: (qmail 41716 invoked by uid 89); 9 Dec 2016 03:38:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=broke, engineers, our X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-pg0-f65.google.com Received: from mail-pg0-f65.google.com (HELO mail-pg0-f65.google.com) (74.125.83.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 03:38:54 +0000 Received: by mail-pg0-f65.google.com with SMTP id p66so774251pga.2; Thu, 08 Dec 2016 19:38:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=C8SFJ/peY5njwdF/oiuWGPGawRG+arfX6v7j8xT5I0M=; b=Y+1WRXf2eHMsfaQOCZ6g6QU3qWbG/l8BujuS6VgIM5yWPSgcstLSqm/4dpT9VmBxRh QsLNpuk03qCBeMElc8Cvl2qhkFvjJpk8UFOwca8uhG/hKsBZIilkpQU84VTYw++ZcwSB v5Lcd8/axfFVeZa7DsCsS7Kdfld7D/25buFr9lnujZJzfEgz2ogSQ1EzKnE0mg8e34lF 87NCphxjcGdzsuqti/WglmQOWSlyH69J1pWf3XzRTikXQETBTBM09HDPQ1MzqkYaV15t edkGXInAowKO3wBpbjlWO1/E50St4L/ALvG+krCm/0apHpsfwBuYlFOKu/r0HoPDB0HM Pbtg== X-Gm-Message-State: AKaTC03yOX04P1/6mrqJz/BWacBnOMp5PiYaXDr3cGL70UPJEeBUCw7pm61/bm/WAkCpuw== X-Received: by 10.98.88.4 with SMTP id m4mr78664948pfb.81.1481254732603; Thu, 08 Dec 2016 19:38:52 -0800 (PST) Received: from bubble.grove.modra.org (CPE-58-160-71-80.tyqh2.lon.bigpond.net.au. [58.160.71.80]) by smtp.gmail.com with ESMTPSA id d197sm53159827pfd.38.2016.12.08.19.38.51 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Dec 2016 19:38:51 -0800 (PST) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 8BDB4C1666; Fri, 9 Dec 2016 14:08:47 +1030 (ACDT) Date: Fri, 9 Dec 2016 14:08:47 +1030 From: Alan Modra To: "Maciej W. Rozycki" Cc: Nick Clifton , binutils@sourceware.org Subject: Re: [binutils-gdb] Fix the linker so that it will not silently generate ELF binaries with invalid program headers. Fix Message-ID: <20161209033847.GN10584@bubble.grove.modra.org> References: <20161123111226.62132.qmail@sourceware.org> <20161208113238.GF10584@bubble.grove.modra.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes On Fri, Dec 09, 2016 at 12:17:31AM +0000, Maciej W. Rozycki wrote: > On Thu, 8 Dec 2016, Alan Modra wrote: > > > > Program Headers: > > > Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align > > > PHDR 0x000034 0x00001034 0x00000000 0x000a0 0x000a0 R E 0x4 > > > INTERP 0x001000 0x00080000 0x00080000 0x00013 0x00013 R 0x1 > > > [Requesting program interpreter: /usr/lib/libc.so.1] > > > LOAD 0x001000 0x00080000 0x00080000 0x00408 0x00408 R E 0x1000 > > > LOAD 0x002000 0x00081000 0x00081000 0x00804 0x00c00 RW 0x1000 > > > DYNAMIC 0x002000 0x00081000 0x00081000 0x00078 0x00078 RW 0x4 > > > > The gABI says: > > > > PT_PHDR > > The array element, if present, specifies the location and size of > > the program header table itself, both in the file and in the > > memory image of the program. This segment type may not occur more > > than once in a file. Moreover, it may occur only if the program > > header table is part of the memory image of the program. If it is > > present, it must precede any loadable segment entry. > > > > The above clearly violates this part of the spec because PT_PHDR is > > present yet is not part of the memory image. > > Well, TBH I think it's all but clear, given that nowhere in the ELF gABI > that I can find there is an actual definition of what "the memory image" > is. I'm not going to argue this point except to note that all specs I've ever read rely on some background knowledge and require interpretation. Those that try to specify endless detail might satisfy lawyers, but often become so unwieldy that they aren't as useful as they should be to engineers. > > Nick's patch forced the first PT_LOAD to cover the program headers. I > > think an equally valid and somewhat better fix would have been to not > > emit PT_PHDR when no PT_LOAD header covers the program headers. The > > reason I say that is because PT_PHDR is optional. A loader can read > > the program headers itself from file using info in the ELF header. > > There may be no loader available to load the program headers or the ELF > file header if a bare-metal ELF image has been externally loaded according > to some interpretation of the headers, and the binary wants to access its > own structures at run time. Existing environments may rely on our current > semantics even if it is not strictly compliant. This is true but I was arguing about the validity of the ELF file. I mentioned a loader only to say what might be done. We do know that on reasonably up to date Linux with glibc, that linking a simple program with a script that leaves no room for headers results in a segfault before Nick's patch, but runs after Nick's patch. My suggestion to remove PHDR (patch attached) results in the same ld.so segfault. It would certainly be useful to know how vxworks behaves with similar tests. We've also found that Nick's patch broke linux kernel builds, which is why I was exploring alternative fixes. -- Alan Modra Australia Development Lab, IBM diff --git a/bfd/elf.c b/bfd/elf.c index 678c043..d7fbca1 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -4507,7 +4507,6 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info) asection *dynsec, *eh_frame_hdr; bfd_size_type amt; bfd_vma addr_mask, wrap_to = 0; - bfd_boolean linker_created_pt_phdr_segment = FALSE; /* Select the allocated sections, and sort them. */ @@ -4560,7 +4559,6 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info) m->p_flags = PF_R | PF_X; m->p_flags_valid = 1; m->includes_phdrs = 1; - linker_created_pt_phdr_segment = TRUE; *pm = m; pm = &m->next; @@ -4612,17 +4610,13 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info) < phdr_size % maxpagesize) || (sections[0]->lma & addr_mask & -maxpagesize) < wrap_to) { + phdr_in_segment = FALSE; /* PR 20815: The ELF standard says that a PT_PHDR segment, if present, must be included as part of the memory image of the program. Ie it must be part of a PT_LOAD segment as well. - If we have had to create our own PT_PHDR segment, but it is - not going to be covered by the first PT_LOAD segment, then - force the inclusion if we can... */ - if ((abfd->flags & D_PAGED) != 0 - && linker_created_pt_phdr_segment) - phdr_in_segment = TRUE; - else - phdr_in_segment = FALSE; + If PHDR won't be part of the memory image, remove it. */ + if (mfirst->p_type == PT_PHDR) + mfirst = mfirst->next; } }