[PATCH] Unconditionally define __stack_chk_guard [BZ #26817]

[Florian Weimer]

* Fāng-ruì Sòng:

> On Mon, Jan 11, 2021 at 9:50 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Fāng-ruì Sòng:
>>
>> > On Mon, Jan 11, 2021 at 1:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>> >>
>> >> * Fangrui Song via Libc-alpha:
>> >>
>> >> > This makes -mstack-protector-guard=global work even if
>> >> > -mstack-protector-guard=tls is the default.
>> >>
>> >> It's unclear how you intend this to work.  Is it just for building
>> >> glibc?  Or also for user code?
>> >
>> > It is for user applications.
>> >
>> > gcc -fstack-protector -mstack-protector-guard=global a.c
>> > or
>> > clang -fstack-protector -mstack-protector-guard=global a.c (clang 12)
>> >
>> > Currently there is an undefined symbol error. This patch fixes the problem.
>>
>> And we really want to use a global data symbol for this?  Nothing else
>> works at the moment, but it seems rather wasteful.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=22850
> For a new thread, nptl/allocatestack.c allocates a memory region, places the
> static TLS block at the end, and uses the remaining space as the thread stack.
> The stack pointer may be just a few hundred bytes below the canary address
> (%fs:0x28 on x86-64) and a large out-of-bounds write can potentially
> override it.

Is this the only reason?

We need to change the TCB allocation anyway.  Unlike LinuxThreads, there
is no technical requirement to have the TCB at the top of the stack.

> This patch will make -mstack-protector-guard=global work, with very
> little overhead (a word) in ld.so.

On the other hand, it may discourage people from enabling the stack
protector because the overhead encountered by instrumented functions
increases.

Perhaps we should add new relocation types and use a hidden, per-DSO
symbol.  The canary value does not have to be the same for all
functions.  I believe OpenBSD does this already.

>> >> In the latter case, we either need to provide a way to initialize a
>> >> hidden __stack_chk_guard symbol (probably with new relocations), or
>> >> export __stack_chk_guard from glibc, under a GLIBC_2.33 symbol version
>> >> for architectures that were missing it before.
>> >
>> > In elf/Versions, __stack_chk_guard is already exported as
>> > __stack_chk_guard@@GLIBC_2.4
>>
>> GLIBC_2.4 is done and dusted, we can't add symbols to it.  New symbols
>> must have symbol version GLIBC_2.33 or later, while preserving the old
>> symbol versions for the ports that already export it.
>>
>> If you run ”make check”, you'll see abilist comparison failures for most
>> ports.  You need to fix those as well.
>
> OK. Added %include "tls.h" to get THREAD_SET_STACK_GUARD,
>
> https://github.com/MaskRay/glibc/tree/stack_chk_guard
> I can post it here if it looks good.

It builds and produces the expected ABI, but the %ifdef kludges are
quite ugly.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill