> Date: Wed, 12 Aug 2015 15:48:57 +0200 (CEST) > From: Mark Kettenis <mark.kette...@xs4all.nl> > > I spent some time today figuting out why the binutils update broke ld > -Z on powerpc. Turns out to be a fairly thorny issue. > > The new binutils discard empty setions. As a result the .gotpad0 and > .gotpad1 sections have disappeared. And a s a consequence the > __got_start and __got_end symbols are now "absolute" symbols as the > section they referenced to is no longer there. For example, an older > libc has: > > 845: 000eeb68 0 NOTYPE GLOBAL DEFAULT 17 __got_start > > whereas -current has: > > 810: 000eeb58 0 NOTYPE GLOBAL DEFAULT ABS __got_start > > On powerpc, crt0.o has weak references to __got_start and __got_end. > When building a binary with ld -Z, these are resolved to the absolute > symbols from libc. At runtime we then use these values, relocated as > if they were addresses within the binary itself, to change protections > and flush the instruction cache. This is very likely to result in a > segmentation fault. > > There is probably a linker bug here, as it doesn't make any sense for > the linker to pick the address of these symbols from libc and stick it > into the binary. But I'm not sure about this. And it isn't all that > obvious what the fix would be. Is the bug that the symbols end up as > absolute? Or is the problem that it sibsequently resolves these to > the values from libc.so? > > It does point out a fundamental weakness about the approach we've > taken with the __plt_start/end and __got_start/end synbols. They work > fine of you use something like dlsym(3) to lookup the value for a > specific object, but if you rely on the default symbol resolution, it > isn't clear if you get the right version. Therefore I think that the > powerpc crt0.o code shouldn't be doing what it is doing. The diff > below removes that code. > > This diff has a downside though. The GOT on -static -nopie binaries > will no longer be read-only. I don't think that is a big loss, as > -static -pie binaries are the default now and those do get a read-only > GOT. > > If we think a read-only GOT for -static binaries is still important, > there are a few other potential options to achive this that don't need > to rely on __got_start/end.
ping? This will also make my life easier with the secure-plt changes that are in the pipeline. > Index: powerpc/md_init.h > =================================================================== > RCS file: /cvs/src/lib/csu/powerpc/md_init.h,v > retrieving revision 1.3 > diff -u -p -r1.3 md_init.h > --- powerpc/md_init.h 26 Dec 2014 13:52:01 -0000 1.3 > +++ powerpc/md_init.h 12 Aug 2015 13:08:28 -0000 > @@ -64,88 +64,20 @@ > #include <sys/syscall.h> /* for SYS_mprotect */ > > #define STR(x) __STRING(x) > + > #define MD_CRT0_START > \ > __asm( > \ > " .text \n" \ > " .section \".text\" \n" \ > " .align 2 \n" \ > -" .size __got_start, 0 \n" \ > -" .type __got_start, @object \n" \ > -" .size __got_end, 0 \n" \ > -" .type __got_end, @object \n" \ > -" .weak __got_start \n" \ > -" .weak __got_end \n" \ > " .globl _start \n" \ > " .type _start, @function \n" \ > " .globl __start \n" \ > " .type __start, @function \n" \ > "_start: \n" \ > "__start: \n" \ > -" # move argument registers to saved registers for startup flush \n" \ > -" # ...except r6 (auxv) as ___start() doesn't need it \n" \ > -" mr %r25, %r3 \n" \ > -" mr %r24, %r4 \n" \ > -" mr %r23, %r5 \n" \ > -" mr %r22, %r7 \n" \ > -" mflr %r27 /* save off old link register */ \n" \ > -" bl 1f \n" \ > -" # this instruction never gets executed but can be used \n" \ > -" # to find the virtual address where the page is loaded. \n" \ > -" bl _GLOBAL_OFFSET_TABLE_@local-4 \n" \ > -"1: \n" \ > -" mflr %r6 # this stores where we are (+4) \n" \ > -" lwz %r18, 0(%r6) # load the instruction at offset_sym \n" \ > -" # it contains an offset to the location \n" \ > -" # of the GOT. \n" \ > -" \n" \ > -" rlwinm %r18,%r18,0,8,30 # mask off the offset portion of the instr. \n" > \ > -" \n" \ > -" /* \n" \ > -" * these adds effectively calculate the value the \n" \ > -" * bl _GLOBAL_OFFSET_TABLE_@local-4 \n" \ > -" * operation that would be below would calculate. \n" \ > -" */ \n" \ > -" add %r28, %r18, %r6 \n" \ > -" \n" \ > -" addi %r3,%r28,4 # calculate the actual got addr \n" \ > -" lwz %r0,__got_start@got(%r3) \n" \ > -" cmpwi %r0,0 \n" \ > -" beq 4f \n" \ > -" cmpw %r0,%r28 \n" \ > -" bne 4f \n" \ > -" lwz %r4,__got_end@got(%r3) \n" \ > -" cmpwi %r4,0 \n" \ > -" beq 2f \n" \ > -" \n" \ > -" sub %r4, %r4, %r0 \n" \ > -" b 3f \n" \ > -"2: \n" \ > -" li %r4, 4 \n" \ > -"3: \n" \ > -" \n" \ > -" /* mprotect GOT to eliminate W+X regions in static binaries */ \n" \ > -" li %r0, " STR(SYS_mprotect) " \n" \ > -" mr %r3, %r28 \n" \ > -" li %r5, 5 /* (PROT_READ|PROT_EXEC) */ \n" \ > -" sc \n" \ > -" \n" \ > -"4: \n" \ > -" li %r0, 0 \n" \ > -" # flush the blrl instruction out of the data cache \n" \ > -" dcbf %r6, %r18 \n" \ > -" sync \n" \ > -" isync \n" \ > -" # make certain that the got table addr is not in the icache \n" \ > -" icbi %r6, %r18 \n" \ > -" sync \n" \ > -" isync \n" \ > -" mtlr %r27 \n" \ > -" # move argument registers back from saved registers \n" \ > -" # putting cleanup in r6 instead of r7 \n" \ > -" mr %r3, %r25 \n" \ > -" mr %r4, %r24 \n" \ > -" mr %r5, %r23 \n" \ > -" mr %r6, %r22 \n" \ > +" # put cleanup in r6 instead of r7 \n" \ > +" mr %r6, %r7 \n" \ > " b ___start \n" \ > ) > > >
