Message ID | CAMe9rOr+qnBVwLwSy-97NKbuxxMDd6M9+8yLpEC2E_hbCN=p5w@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Mon, Jul 10, 2017 at 10:34 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Jul 10, 2017 at 6:29 AM, Nick Alcock <nix@esperi.org.uk> wrote: >> On 10 Jul 2017, H. J. Lu said: >>> On Mon, Jul 10, 2017 at 11:50:02AM +0100, Nick Alcock wrote: >>>> If it passes a test build with --enable-stack-protector=all without >>>> pulling junk into ld.so and exploding at ld.so link time, sure. (That's >>>> what happened every time I tried to remove this stuff before, but I may >>>> have failed to notice that this may not be necessary any more.) >>> >>> Here is the updated patch. tst-_dl_addr_inside_object should be >>> linked with $(dummy-stack-chk-fail). Otherwise, __stack_chk_fail is >>> undefined. OK for master branch? >> >> I presume this is because it's in $(all-nonlib)? Are other all-nonlib >> things similarly affected? (If they are, is the code in Makerules >> perhaps a better place to adjust this?) >> >> I guess the only affected nonlib things would be things that directly >> link against objects that will otherwise end up in the shared libc or >> ld.so, which means this is the only affected test (since only those >> things reference the usually-hidden __stack_chk_fail). If so, I have no >> objection to this patch, but maybe a comment explaining this would be a >> good idea so that if more such tests turn up in future this unusual >> piece of linking can be generalized a bit. >> >> Modulo that, I have no objections, but of course I also have no actual >> right to ack anything whatsoever :) >> >>>> > -/* On some architectures, this helps needless PIC pointer setup >>>> > - that would be needed just for the __stack_chk_fail call. */ >>>> >>>> Does anyone know what architectures these might be? Presumably x86-32... >>>> >>> >>> config/i386/i386.c: __stack_chk_fail_local hidden function instead of calling >> >> I presume you tested a build on x86-32 :) I guess that will suffice... > > We must keep debug/stack_chk_fail_local.c for libc_nonshared.a. > Here is the updated patch to add __stack_chk_fail_local alias only > to libc.so. > > Tested on i686 and x86-64 with and without --enable-stack-protector=all. > OK for master? > Any objections? https://sourceware.org/ml/libc-alpha/2017-07/msg00406.html
From 217275043ad73f922d07f42e5fc1a4be70183209 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 9 Jul 2017 08:39:17 -0700 Subject: [PATCH] Add __stack_chk_fail_local alias only to libc.so [BZ #21740] commit 524a8ef2ad76af8ac049293d993a1856b0d888fb Author: Nick Alcock <nick.alcock@oracle.com> Date: Mon Dec 26 10:08:57 2016 +0100 PLT avoidance for __stack_chk_fail [BZ #7065] Add a hidden __stack_chk_fail_local alias to libc.so, and make sure that on targets which use __stack_chk_fail, this does not introduce a local PLT reference into libc.so. unconditionally added strong_alias (__stack_chk_fail, __stack_chk_fail_local) Since libc.a and libc_nonshared.a has debug/stack_chk_fail_local.c, __stack_chk_fail_local alias should be limited to libc.so. Tested on x86-64 with --enable-stack-protector=all and got FAIL: elf/tst-env-setuid FAIL: elf/tst-env-setuid-tunables FAIL: stdlib/tst-secure-getenv which are the same as without this patch. [BZ #21740] * debug/stack_chk_fail.c (__stack_chk_fail_local): Add the alias only if SHARED is defined. --- debug/stack_chk_fail.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/debug/stack_chk_fail.c b/debug/stack_chk_fail.c index 4f73464..120d269 100644 --- a/debug/stack_chk_fail.c +++ b/debug/stack_chk_fail.c @@ -28,4 +28,8 @@ __stack_chk_fail (void) __fortify_fail ("stack smashing detected"); } +#ifdef SHARED +/* Some targets call __stack_chk_fail_local as a hidden function within + libc.so. */ strong_alias (__stack_chk_fail, __stack_chk_fail_local) +#endif -- 2.9.4