Remove debug/stack_chk_fail_local.c [BZ #21740]
Commit Message
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?
Comments
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(+)
@@ -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