From owner-svn-src-head@freebsd.org Tue Oct 30 22:24:11 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BFD9C1008C95 for ; Tue, 30 Oct 2018 22:24:11 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: from mail-yb1-f181.google.com (mail-yb1-f181.google.com [209.85.219.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5DEE37D59D for ; Tue, 30 Oct 2018 22:24:11 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: by mail-yb1-f181.google.com with SMTP id p144-v6so5755685yba.11 for ; Tue, 30 Oct 2018 15:24:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MfLbjGeyEJ1KA2rXhvmlXdBFx88g8q6LsqliAAZq1xQ=; b=YnfYYHfzCvVwTXQxf9vNgqFCFGlllTPzjJBUQronACSytc9vzrO6gS0lAaQlZncCHD nsShwbavhsLz6bNT9EfbO5/KiSlxQVF+fLGoeQMPyZ2pIFWlQdvY14c5cp51qWXN91T6 h44ODl/dQUuVLQ+4Y2HTAAPuxZL75KEDy++Fjp7foKIm1h8zn+8+lJXxW9fMH5Q0j4b7 ClcZBrW4SfXQ8UYo5rsUiUoJVCEcXxqyngzSyJU74mM2kUBT/E+zBGLasU7wsbWiCPGA z1LTpOWoCJVRQpuyhI3ME2KhJ5CLMHJIJTswCquWyaPvCBnCMXgJzqwi2rBGx+TymVQ7 AKWA== X-Gm-Message-State: AGRZ1gL1EK1wxiK/K7Huelp2/QuEmHqbQ9KOUbOFy0zoQgSv5yhGZIst Aos0wrC9970Iz7uO8OObjX4IK7fYAGM= X-Google-Smtp-Source: AJdET5dQNtkr4UIdy1QYA5bnJjOUSZvlCJhRJHcflJk9sNha01RHMbbtNxPS9X+UFLw1niHinuRJvA== X-Received: by 2002:a25:ad9a:: with SMTP id z26-v6mr619987ybi.267.1540938245109; Tue, 30 Oct 2018 15:24:05 -0700 (PDT) Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com. [209.85.219.169]) by smtp.gmail.com with ESMTPSA id 138-v6sm6122702yws.28.2018.10.30.15.24.04 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 Oct 2018 15:24:04 -0700 (PDT) Received: by mail-yb1-f169.google.com with SMTP id i78-v6so5778223ybg.0 for ; Tue, 30 Oct 2018 15:24:04 -0700 (PDT) X-Received: by 2002:a25:6d44:: with SMTP id i65-v6mr628848ybc.454.1540938244538; Tue, 30 Oct 2018 15:24:04 -0700 (PDT) MIME-Version: 1.0 References: <7DC6D9C4-C153-4BCE-851C-22C890AB0D73@yahoo.com> <2D7C5FC6-CA8D-4AAB-BD64-CC883531F737@yahoo.com> In-Reply-To: <2D7C5FC6-CA8D-4AAB-BD64-CC883531F737@yahoo.com> From: Alexander Richardson Date: Tue, 30 Oct 2018 22:23:52 +0000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r339876 - head/libexec/rtld-elf To: marklmi26-fbsd@yahoo.com Cc: svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Oct 2018 22:24:12 -0000 On Tue, 30 Oct 2018 at 22:16, Mark Millard wrote: > > On 2018-Oct-30, at 2:40 PM, Alexander Richardson wrote: > > > On Tue, 30 Oct 2018 at 21:32, Mark Millard wrote: > >> > >> > >> > >> On 2018-Oct-30, at 2:23 PM, Alexander Richardson wrote: > >> > >>> On Tue, 30 Oct 2018 at 18:19, Mark Millard wrote: > >>>> > >>>> Alexander Richardson arichardson at freebsd.org wrote on > >>>> Tue Oct 30 15:33:00 UTC 2018 : > >>>> > >>>>> On Tue, 30 Oct 2018 at 10:17, Michael Tuexen > >>>>> wrote: > >>>>>> > >>>>>>> On 29. Oct 2018, at 22:08, Alex Richardson wrote: > >>>>>>> > >>>>>>> Author: arichardson > >>>>>>> Date: Mon Oct 29 21:08:02 2018 > >>>>>>> New Revision: 339876 > >>>>>>> URL: https://svnweb.freebsd.org/changeset/base/339876 > >>>>>>> > >>>>>>> Log: > >>>>>>> rtld: set obj->textsize correctly > >>>>>>> > >>>>>>> With lld-generated binaries the first PT_LOAD will usually be a read-only > >>>>>>> segment unless you pass --no-rosegment. For those binaries the textsize is > >>>>>>> determined by the next PT_LOAD. To allow both LLD and bfd 2.17 binaries to > >>>>>>> be parsed correctly use the end of the last PT_LOAD that is marked as > >>>>>>> executable instead. > >>>>>>> > >>>>>>> I noticed that the value was wrong while adding some debug prints for some rtld > >>>>>>> changes for CHERI binaries. `obj->textsize` only seems to be used by PPC so the > >>>>>>> effect is untested. However, the value before was definitely wrong and the new > >>>>>>> result matches the phdrs. > >>>>>> I build kernel and world with a revision later than this on a PPC. Buildword > >>>>>> ends up with a world where almost all binaries are segfaulting.... Especially gdb > >>>>>> (but svn, ls or so all segfault). > >>>>>> > >>>>>> Best regards > >>>>>> Michael > >>>>> > >>>>> This is rather surprising since if anything the range of the icache > >>>>> flush should increase rather than decrease after this change. > >>>>> > >>>>> I can only see this causing a behaviour change if we actually need to > >>>>> flush more than just the executable segments. > >>>>> Is it possible that some binary/library contains a non-executable > >>>>> segment as the first PT_LOAD? > >>>>> Or is there some linker script that adds custom PHDRS? > >>>> > >>>> The following is based on using devel/powerpc64-xtoolchain-gcc > >>>> to buildworld buildkernel on/for powerpc64. (I experiment with > >>>> using fairly modern tools to target powerpc64 and powerpc.) > >>>> The build context is head -r339076 based, both for what > >>>> did the build and for what it was building. > >>>> > >>>> I report from both elfdump and objdump output > >>>> because each seems to have some oddities in what > >>>> it outputs. > >>>> > >>>> I start with elfdump (which leaves sh_flags blank > >>>> and shows a section header with sh_name empty > >>>> that objdump does not list at all): > >>>> > >>>> # elfdump -pc /bin/ls | less > >>>> > >>>> . . . > >>>> > >>>> As for objdump on the same file (section > >>>> one less than elfdump listed, no empty sh_name > >>>> section listed): > >>>> > >>>> # objdump -ph /bin/ls | less > >>>> > >>>> /bin/ls: file format elf64-powerpc-freebsd > >>>> > >>>> Program Header: > >>>> PHDR off 0x0000000000000040 vaddr 0x0000000010000040 paddr 0x0000000010000040 align 2**3 > >>>> filesz 0x0000000000000188 memsz 0x0000000000000188 flags r-- > >>>> INTERP off 0x00000000000001c8 vaddr 0x00000000100001c8 paddr 0x00000000100001c8 align 2**0 > >>>> filesz 0x0000000000000015 memsz 0x0000000000000015 flags r-- > >>>> LOAD off 0x0000000000000000 vaddr 0x0000000010000000 paddr 0x0000000010000000 align 2**16 > >>>> filesz 0x000000000000910c memsz 0x000000000000910c flags r-x > >>>> LOAD off 0x0000000000009110 vaddr 0x0000000010019110 paddr 0x0000000010019110 align 2**16 > >>>> filesz 0x0000000000000ee0 memsz 0x00000000000010e8 flags rw- > >>>> DYNAMIC off 0x0000000000009138 vaddr 0x0000000010019138 paddr 0x0000000010019138 align 2**3 > >>>> filesz 0x00000000000001c0 memsz 0x00000000000001c0 flags rw- > >>>> NOTE off 0x00000000000001e0 vaddr 0x00000000100001e0 paddr 0x00000000100001e0 align 2**2 > >>>> filesz 0x0000000000000030 memsz 0x0000000000000030 flags r-- > >>>> STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**4 > >>>> filesz 0x0000000000000000 memsz 0x0000000000000000 flags rw- > >>>> > >>>> Dynamic Section: > >>>> NEEDED libutil.so.9 > >>>> NEEDED libncursesw.so.8 > >>>> NEEDED libc.so.7 > >>>> INIT 0x0000000010019328 > >>>> FINI 0x0000000010019340 > >>>> HASH 0x0000000010000210 > >>>> STRTAB 0x0000000010000d00 > >>>> SYMTAB 0x0000000010000490 > >>>> STRSZ 0x000000000000035a > >>>> SYMENT 0x0000000000000018 > >>>> DEBUG 0x0000000000000000 > >>>> PLTGOT 0x0000000010019898 > >>>> PLTRELSZ 0x00000000000006f0 > >>>> PLTREL 0x0000000000000007 > >>>> JMPREL 0x00000000100012f8 > >>>> 0x70000000 0x00000000100089b4 > >>>> RELA 0x0000000010001160 > >>>> RELASZ 0x0000000000000198 > >>>> RELAENT 0x0000000000000018 > >>>> VERNEED 0x0000000010001110 > >>>> VERNEEDNUM 0x0000000000000001 > >>>> VERSYM 0x000000001000105a > >>>> > >>>> Version References: > >>>> required from libc.so.7: > >>>> 0x077a28b3 0x00 05 FBSD_1.3 > >>>> 0x077a28b1 0x00 04 FBSD_1.1 > >>>> 0x077a28b5 0x00 03 FBSD_1.5 > >>>> 0x077a28b0 0x00 02 FBSD_1.0 > >>>> private flags = 0x1: [abiv1] > >>>> > >>>> Sections: > >>>> Idx Name Size VMA LMA File off Algn > >>>> 0 .interp 00000015 00000000100001c8 00000000100001c8 000001c8 2**0 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 1 .note.tag 00000030 00000000100001e0 00000000100001e0 000001e0 2**2 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 2 .hash 0000027c 0000000010000210 0000000010000210 00000210 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 3 .dynsym 00000870 0000000010000490 0000000010000490 00000490 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 4 .dynstr 0000035a 0000000010000d00 0000000010000d00 00000d00 2**0 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 5 .gnu.version 000000b4 000000001000105a 000000001000105a 0000105a 2**1 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 6 .gnu.version_r 00000050 0000000010001110 0000000010001110 00001110 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 7 .rela.dyn 00000198 0000000010001160 0000000010001160 00001160 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 8 .rela.plt 000006f0 00000000100012f8 00000000100012f8 000012f8 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 9 .init 0000002c 00000000100019f0 00000000100019f0 000019f0 2**4 > >>>> CONTENTS, ALLOC, LOAD, READONLY, CODE > >>>> 10 .text 00007204 0000000010001a20 0000000010001a20 00001a20 2**5 > >>>> CONTENTS, ALLOC, LOAD, READONLY, CODE > >>>> 11 .fini 00000024 0000000010008c30 0000000010008c30 00008c30 2**4 > >>>> CONTENTS, ALLOC, LOAD, READONLY, CODE > >>>> 12 .rodata 000004b0 0000000010008c58 0000000010008c58 00008c58 2**3 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 13 .eh_frame 00000004 0000000010009108 0000000010009108 00009108 2**2 > >>>> CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>> 14 .ctors 00000010 0000000010019110 0000000010019110 00009110 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 15 .dtors 00000010 0000000010019120 0000000010019120 00009120 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 16 .jcr 00000008 0000000010019130 0000000010019130 00009130 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 17 .dynamic 000001c0 0000000010019138 0000000010019138 00009138 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 18 .opd 00000468 00000000100192f8 00000000100192f8 000092f8 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 19 .got 00000098 0000000010019800 0000000010019800 00009800 2**8 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 20 .plt 00000708 0000000010019898 0000000010019898 00009898 2**3 > >>>> ALLOC > >>>> 21 .data 00000050 0000000010019fa0 0000000010019fa0 00009fa0 2**3 > >>>> CONTENTS, ALLOC, LOAD, DATA > >>>> 22 .bss 00000208 0000000010019ff0 0000000010019ff0 00009ff0 2**3 > >>>> ALLOC > >>>> 23 .comment 000002b5 0000000000000000 0000000000000000 00009ff0 2**0 > >>>> CONTENTS, READONLY > >>>> 24 .gnu_debuglink 00000010 0000000000000000 0000000000000000 0000a2a8 2**2 > >>>> CONTENTS, READONLY > >>>> > >>>> > >>> > >>> The first PT_LOAD is also executable so there will be no be behaviour > >>> change. It must be one of the library dependencies that is different. > >>> The difference in section headers output doesn't matter here since all > >>> that RTLD looks at is the PHDRS. > >>> > >> > >> I think you missed the fact that the .got and .plt are in the > >> material from the 2nd PT_LOAD: there are writeable code areas > >> involved for powerpc64. > >> > >> So both PT_LOAD's contribute code areas and there is a space > >> between the two contributions. The first PT_LOAD loads most > >> code (the readonly code, to be specific). The 2nd contributes > >> a writable-code area. > >> > >> But may be i've misunderstood the assumptions that your change > >> is based on. > > > > Before my change the second PT would also not be included in > > obj->textsize since we were setting obj->textsize as end of > > PT_LOAD[0]. > > After the change it will be end of last executable PT_LOAD (which will > > usually be PT_LOAD[0] but could also include more now). > > Since there is only one executable PT_LOAD which happens to be also be > > PT_LOAD[0], there will be no change to obj->textsize. > > > > The only time this commit could change anything is if there is a > > PT_LOAD that is executable but is not PT_LOAD[0] or if PT_LOAD[0] is > > not executable. > > I think it must be the former case since as far as I know ld.bfd will > > always create a read/execute segment as PT_LOAD[0] unless it is given > > a custom linker script. > > I do not see any evidence that PF_X would match the tests for either > of the following from the 2nd PT_LOAD: > > 19 .got 00000098 0000000010019800 0000000010019800 00009800 2**8 > CONTENTS, ALLOC, LOAD, DATA > 20 .plt 00000708 0000000010019898 0000000010019898 00009898 2**3 > ALLOC > > in the likes of: > > 135 if ((segs[nsegs]->p_flags & PF_X) == PF_X) { > 136 text_end = MAX(text_end, > 137 round_page(segs[nsegs]->p_vaddr + segs[nsegs]->p_memsz)); > 138 } > > in map_object.c > > This is possibly to avoid: > > 467 if (elfflags & PF_X) > 468 prot |= PROT_EXEC; > > because of writable code. > > > rtld.c also, for: > > 1397 nsegs++; > 1398 if ((ph->p_flags & PF_X) == PF_X) { > 1399 obj->textsize = MAX(obj->textsize, > 1400 round_page(ph->p_vaddr + ph->p_memsz) - obj->vaddrbase); > 1401 } > > Still, I may be missing something in my understanding. > Before this change obj->textsize was always set as the end of PT_LOAD[0]. Now it will contain everything up to the end of the last PT_LOAD with execute permissions. In the binary you dumped this is PT_LOAD[0] both before and after the patch so there is no change in behaviour. The .got and .plt we not included in textsize before this patch either. Therefore the only way I can see this patch breaking anything is if the PHDRS of one of the loaded libraries causes a change in behaviour. Alex