[08/12] De-PLTize __stack_chk_fail internal calls within libc.so.

Message ID 3785c2b6-4d2b-e1ac-4276-8a6231db7c56@redhat.com
State New
Headers show

Commit Message

Florian Weimer Dec. 16, 2016, 11:39 a.m.
On 12/15/2016 09:22 PM, Florian Weimer wrote:
> On 12/15/2016 09:00 PM, Nix wrote:

>

>>> Could you try this?

>>>

>>> # if defined STACK_PROTECTOR_LEVEL && STACK_PROTECTOR_LEVEL > 0

>>> asm (".hidden __stack_chk_fail_local");

>>> asm ("__stack_chk_fail = __stack_chk_fail_local");

>>> # endif

>>

>> No change :( the only reference to __stack_chk_fail is still inside

>> stack_chk_fail_local:

>>

>> Symbols from libc_pic.a[libc-stack_chk_fail_local.os]:

>>

>> Name                        Value            Class  Type

>> Size             Line Section

>>

>> __GI_memcpy                ||GLOBAL|NOTYPE  ||    |UNDEF

>> __GI_memmove               ||GLOBAL|NOTYPE  ||    |UNDEF

>> __GI_memset                ||GLOBAL|NOTYPE  ||    |UNDEF

>> __stack_chk_fail           ||GLOBAL|NOTYPE  ||    |UNDEF

>> __stack_chk_fail_local     |0000000000000000|GLOBAL|FUNC

>> |0000000000000010|    |.text

>> libc-stack_chk_fail_local.c|0000000000000000|LOCAL |FILE

>> |0000000000000000|    |ABS

>>

>> (And, of course, this code is not affected by your suggestion, because

>> it's compiled with -fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0.)

>

> I think this attempt at PLT avoidance within libc.so itself is subtly

> wrong.  We need to mirror more closely what

> libc_hidden_proto/libc_hidden_def does, and perhaps disentangle this

> from the __stack_chk_fail_local definition used in other DSOs.

>

> I think this means removing any definition of a C function definition

> called __stack_chk_fail_local from libc.so, and instead use a strong

> alias from __stack_chk_fail to __stack_chk_fail_local to define the

> symbol.  The alias will not incorporate a PLT reference.  If you look at

> include/libc-symbols.h, strong_alias and hidden_def are quite similar.


With this patch on top of the series you posted (without any other 
changes), I get both MIPS and ia64 to build, and I don't see a PLT 
failure on SPARC.

Thanks,
Florian

Comments

Nix Dec. 16, 2016, 11:44 a.m. | #1
On 16 Dec 2016, Florian Weimer outgrape:

> On 12/15/2016 09:22 PM, Florian Weimer wrote:

>> I think this attempt at PLT avoidance within libc.so itself is subtly

>> wrong.  We need to mirror more closely what

>> libc_hidden_proto/libc_hidden_def does, and perhaps disentangle this

>> from the __stack_chk_fail_local definition used in other DSOs.

>>

>> I think this means removing any definition of a C function definition

>> called __stack_chk_fail_local from libc.so, and instead use a strong

>> alias from __stack_chk_fail to __stack_chk_fail_local to define the

>> symbol.  The alias will not incorporate a PLT reference.  If you look at

>> include/libc-symbols.h, strong_alias and hidden_def are quite similar.

>

> With this patch on top of the series you posted (without any other changes), I get both MIPS and ia64 to build, and I don't see a

> PLT failure on SPARC.


I'll give this a spin, then post a new series.

It'll be a few hours, as usual for a full test cycle :(

-- 
NULL && (void)

Patch hide | download patch | download mbox

diff --git a/debug/Makefile b/debug/Makefile
index 27da081..cfcf392 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -48,14 +48,10 @@  routines  = backtrace backtracesyms backtracesymsfd noophooks \
 	    vdprintf_chk obprintf_chk \
 	    longjmp_chk ____longjmp_chk \
 	    fdelt_chk poll_chk ppoll_chk \
-	    stack_chk_fail fortify_fail libc-stack_chk_fail_local \
+	    stack_chk_fail fortify_fail \
 	    $(static-only-routines)
 
-# stack_chk_fail_local must be non-PIC, thus static-only, but we also
-# want an identical thunk hidden in libc.so to avoid going via the PLT.
-
 static-only-routines := warning-nop stack_chk_fail_local
-shared-only-routines += libc-stack_chk_fail_local
 
 # Building the stack-protector failure routines with stack protection
 # makes no sense.
diff --git a/debug/libc-stack_chk_fail_local.c b/debug/libc-stack_chk_fail_local.c
deleted file mode 100644
index 73da970..0000000
--- a/debug/libc-stack_chk_fail_local.c
+++ /dev/null
@@ -1,3 +0,0 @@ 
-/* This goes into the shared libc.  */
-
-#include <stack_chk_fail_local.c>
diff --git a/debug/stack_chk_fail.c b/debug/stack_chk_fail.c
index 4d0796f..a545239 100644
--- a/debug/stack_chk_fail.c
+++ b/debug/stack_chk_fail.c
@@ -27,3 +27,11 @@  __stack_chk_fail (void)
 {
   __fortify_fail ("stack smashing detected");
 }
+
+#ifdef SHARED
+/* The compiler generates hidden references to __stack_chk_fail for
+   PLT avoidance.  Outside of libc.so, a definition provided by
+   libc_nonshared.a, but within libc.so, a local definition is
+   needed.  */
+strong_alias (__stack_chk_fail, __stack_chk_fail_local)
+#endif
diff --git a/sysdeps/generic/symbol-hacks.h b/sysdeps/generic/symbol-hacks.h
index 36908b5..ce576c9 100644
--- a/sysdeps/generic/symbol-hacks.h
+++ b/sysdeps/generic/symbol-hacks.h
@@ -4,8 +4,4 @@ 
 asm ("memmove = __GI_memmove");
 asm ("memset = __GI_memset");
 asm ("memcpy = __GI_memcpy");
-
-/* -fstack-protector generates calls to __stack_chk_fail, which need
-   similar adjustments to avoid going through the PLT.  */
-asm ("__stack_chk_fail = __stack_chk_fail_local");
 #endif