[musl] musl, glibc and ideal place for __stack_chk_fail_local

[musl] musl, glibc and ideal place for __stack_chk_fail_local

[Sergei Trofimovich]

[ sending it to musl, glibc and gcc devel mailing list as we need
  to build a consensus across the projects ]

To support smash stack protection gcc emits __stack_chk_fail
calls on all targets. On top of that gcc emits __stack_chk_fail_local
calls at least on i386 and powerpc:
    https://bugs.gentoo.org/706210#c9

gcc can either use libssp/libssp_nonshared fallback or rely on libc
to provide __stack_chk_fail. Where ideally should gcc pick
__stack_chk_fail_local?

Looks like gcc/glibc and musl disagree on that:
- gcc: gcc either provides it from libssp_nonshared.a if libc has
  no ssp support or pulls it from libc
- glibc: provides both __stack_chk_fail (deault) and
  __stack_chk_fail_local (avoid PLT) symbols.
  __stack_chk_fail_local comes from libc_nonshared.a and is
  added to linker script as: $ cat /usr/lib/libc.so
    OUTPUT_FORMAT(elf32-i386)
    GROUP ( /lib/libc.so.6 /usr/lib/libc_nonshared.a  AS_NEEDED ( /lib/ld-linux.so.2 ) )
- musl: provides only __stack_chk_fail (default) and
  refuses to provide __stack_chk_fail_local:
  https://www.openwall.com/lists/musl/2018/09/11/2

This makes musl effectively not support ssp on i386 and probably powerpc.

Currently gcc's assumption is that musl supports ssp symbols
from libc on all targets:
   https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/configure.ac;h=a7521ee99436a7c12159bdde0471dc66d3c4288e;hb=HEAD#l6079

  6088     case "$target" in
  6089        *-*-musl*)
  6090          # All versions of musl provide stack protector
  6091          gcc_cv_libc_provides_ssp=yes;;

Clearly that assumption is not correct as __stack_chk_fail_local
is not provided by musl and linking fails.

This sounds like a expectation mismatch between gcc and musl
of what it takes to implement an ssp interface.

What should we do to make it fixed long term and short term?

Long term:

Is there a vision of perfect end state agreed with gcc/glibc/musl
folk so we could just implement it? If there is none let's try to
form one.

My understanding of requirements for libc that exposes ssp support:
- __stack_chk_fail is implemented as a default symbol
- __stack_chk_fail_local is implemented as a local symbol to avoid PLT.
  (Why is it important? To avoid use of potentially already broken stack?)

My understanding of possible perfect end state:
1. All libcs are required to somehow provide both __stack_chk_fail
   and __stack_chk_fail_local: be it linker script, crt*.o files or an extra
   libc_nonshared.a which gcc explicitly uses. Which one is best?
2. All libcs are required to provide only __stack_chk_fail and gcc always
   provides __stack_chk_local from libgcc.a, or from new libgcc_ssp.a.
   Evntually glibc drops it's __stack_chk_fail definition.
3. Your variant.

How do you gcc/glibc/musl folk see it? Once we decide I'll file bugs
against agreed projects. At least gcc could explicitly document the
interface.

Short term:

While the above is not addressed what should we do about musl in gcc?

Should gcc stop trying use musl on i386/powerpc here:
  6088     case "$target" in
  6089        *-*-musl*)
  6090          # All versions of musl provide stack protector
  6091          gcc_cv_libc_provides_ssp=yes;;
and fall back to libssp instead?

If it makes sense I'll create a bug against gcc.

Thanks!

-- 

  Sergei

Re: [musl] musl, glibc and ideal place for __stack_chk_fail_local

[Rich Felker]

On Sat, Jan 25, 2020 at 10:53:31AM +0000, Sergei Trofimovich wrote:
> [ sending it to musl, glibc and gcc devel mailing list as we need
>   to build a consensus across the projects ]
> 
> To support smash stack protection gcc emits __stack_chk_fail
> calls on all targets. On top of that gcc emits __stack_chk_fail_local
> calls at least on i386 and powerpc:
>     https://bugs.gentoo.org/706210#c9
> 
> gcc can either use libssp/libssp_nonshared fallback or rely on libc
> to provide __stack_chk_fail. Where ideally should gcc pick
> __stack_chk_fail_local?

*Ideally*, GCC would emit it as a linkonce function section in every
file that used it, the same way it does for __x86.get_pc_thunk.bx,
etc. But that's not going to happen in old GCC versions so it's not a
real solution even if new GCC were improved to do that.

Second-best would be for __stack_chk_fail_local to be in the
static-only part of libgcc. It's the same kind of compiler glue. This
is plausible to patch into any GCC version when building.

In reality, the path of least resistance is just continuing to have it
in libssp_nonshared.a; this is what all the distros do. All it
requires is patching the GCC specs so that -lssp_nonshared is always
passed.

> Looks like gcc/glibc and musl disagree on that:
> - gcc: gcc either provides it from libssp_nonshared.a if libc has
>   no ssp support or pulls it from libc
> - glibc: provides both __stack_chk_fail (deault) and
>   __stack_chk_fail_local (avoid PLT) symbols.
>   __stack_chk_fail_local comes from libc_nonshared.a and is
>   added to linker script as: $ cat /usr/lib/libc.so
>     OUTPUT_FORMAT(elf32-i386)
>     GROUP ( /lib/libc.so.6 /usr/lib/libc_nonshared.a  AS_NEEDED ( /lib/ld-linux.so.2 ) )

There's no good reason for it to come from a "nonshared" lib file
provided by libc rather than by the compiler. It's compiler ABI glue
between the public API/ABI (__stack_chk_fail) and the PIC callers that
don't want to be burdened by the GOT pointer setup to be able to call
thru PLT.

> - musl: provides only __stack_chk_fail (default) and
>   refuses to provide __stack_chk_fail_local:
>   https://www.openwall.com/lists/musl/2018/09/11/2

It's not that we refuse; it's that we can't, at least not without
gratuitously changing other things. libc.so can't be made a linker
script because then the real .so would have a different name and the
DT_NEEDED would thereby change. (No, libc.so can't use DT_SONAME to
control the name because then if you install /lib/ld-musl-$(ARCH).so.1
on a glibc-based system and run ldconfig, ldconfig will forcibly ln -s
ld-musl-$(ARCH).so.1 /lib/libc.so, which is A Bad Thing.)

There is a half-serious proposal to put it in crti.o which is always
linked too, but that seems like an ugly hack to me...

> This makes musl effectively not support ssp on i386 and probably powerpc.

It's working for all dists because we/they just make GCC pass
-lssp_nonshared via specs.

> Currently gcc's assumption is that musl supports ssp symbols
> from libc on all targets:
>    https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/configure.ac;h=a7521ee99436a7c12159bdde0471dc66d3c4288e;hb=HEAD#l6079
> 
>   6088     case "$target" in
>   6089        *-*-musl*)
>   6090          # All versions of musl provide stack protector
>   6091          gcc_cv_libc_provides_ssp=yes;;
> 
> Clearly that assumption is not correct as __stack_chk_fail_local
> is not provided by musl and linking fails.
> 
> This sounds like a expectation mismatch between gcc and musl
> of what it takes to implement an ssp interface.
> 
> What should we do to make it fixed long term and short term?
> 
> Long term:
> 
> Is there a vision of perfect end state agreed with gcc/glibc/musl
> folk so we could just implement it? If there is none let's try to
> form one.
> 
> My understanding of requirements for libc that exposes ssp support:
> - __stack_chk_fail is implemented as a default symbol
> - __stack_chk_fail_local is implemented as a local symbol to avoid PLT.
>   (Why is it important? To avoid use of potentially already broken stack?)

Because performance cost of -fstack-protector would go from 1-2% up to
5-10% on i386 and other archs where PLT contract requires a GOT
register, since loading the GOT register is expensive
(__x86.get_pc_thunk.* thunk itself is somewhat costly, and you throw
away one of only a small number of available registers, increasing
register pressure and hurting codegen).

> My understanding of possible perfect end state:
> 1. All libcs are required to somehow provide both __stack_chk_fail
>    and __stack_chk_fail_local: be it linker script, crt*.o files or an extra
>    libc_nonshared.a which gcc explicitly uses. Which one is best?
> 2. All libcs are required to provide only __stack_chk_fail and gcc always
>    provides __stack_chk_local from libgcc.a, or from new libgcc_ssp.a.
>    Evntually glibc drops it's __stack_chk_fail definition.
> 3. Your variant.
> 
> How do you gcc/glibc/musl folk see it? Once we decide I'll file bugs
> against agreed projects. At least gcc could explicitly document the
> interface.
> 
> Short term:
> 
> While the above is not addressed what should we do about musl in gcc?
> 
> Should gcc stop trying use musl on i386/powerpc here:
>   6088     case "$target" in
>   6089        *-*-musl*)
>   6090          # All versions of musl provide stack protector
>   6091          gcc_cv_libc_provides_ssp=yes;;
> and fall back to libssp instead?

Absolutely not. libssp is unsafe and creates new vulns/attack surface
by doing introspective stuff after the process is already *known to
be* in a compromised state. It should never be used. musl's
__stack_chk_fail is safe and terminates immediately.

The short term solution is just to continue what we're doing, adding
-lssp_nonshared to specs.

Ideally, though, GCC would just emit the termination inline (or at
least have an option to do so) rather than calling __stack_chk_fail or
the local version. This would additionally harden against the case
where the GOT is compromised.

Rich

Re: [musl] musl, glibc and ideal place for __stack_chk_fail_local

[Segher Boessenkool]

On Sat, Jan 25, 2020 at 10:54:24AM -0500, Rich Felker wrote:
> > To support smash stack protection gcc emits __stack_chk_fail
> > calls on all targets. On top of that gcc emits __stack_chk_fail_local
> > calls at least on i386 and powerpc:

(Only on 32-bit -fPIC -msecure-plt, for Power).

> There is a half-serious proposal to put it in crti.o which is always
> linked too, but that seems like an ugly hack to me...

Not *very* ugly, but it would be very effective, and no real downsides
to it (or do you see something?)

> > My understanding of requirements for libc that exposes ssp support:
> > - __stack_chk_fail is implemented as a default symbol
> > - __stack_chk_fail_local is implemented as a local symbol to avoid PLT.
> >   (Why is it important? To avoid use of potentially already broken stack?)
> 
> Because performance cost of -fstack-protector would go from 1-2% up to
> 5-10% on i386 and other archs where PLT contract requires a GOT
> register, since loading the GOT register is expensive
> (__x86.get_pc_thunk.* thunk itself is somewhat costly, and you throw
> away one of only a small number of available registers, increasing
> register pressure and hurting codegen).

On Power it is just the setting up itself that is costly (in the config
where we have this _local thing).

> Absolutely not. libssp is unsafe and creates new vulns/attack surface
> by doing introspective stuff after the process is already *known to
> be* in a compromised state. It should never be used. musl's
> __stack_chk_fail is safe and terminates immediately.

Some implementations even print strings from the stack, it can be worse ;-)

> Ideally, though, GCC would just emit the termination inline (or at
> least have an option to do so) rather than calling __stack_chk_fail or
> the local version. This would additionally harden against the case
> where the GOT is compromised.

Yeah, but how to terminate is system-specific, it's much easier to punt
this job to the libc to do ;-)

Open a GCC PR for this please?


Segher

Re: [musl] musl, glibc and ideal place for __stack_chk_fail_local

[Rich Felker]

On Thu, Jan 30, 2020 at 06:33:51AM -0600, Segher Boessenkool wrote:
> On Sat, Jan 25, 2020 at 10:54:24AM -0500, Rich Felker wrote:
> > > To support smash stack protection gcc emits __stack_chk_fail
> > > calls on all targets. On top of that gcc emits __stack_chk_fail_local
> > > calls at least on i386 and powerpc:
> 
> (Only on 32-bit -fPIC -msecure-plt, for Power).

Right, but musl only supports the secure-plt ABI.

> > There is a half-serious proposal to put it in crti.o which is always
> > linked too, but that seems like an ugly hack to me...
> 
> Not *very* ugly, but it would be very effective, and no real downsides
> to it (or do you see something?)

Well either the thunk has to be written in asm per-arch, or some ld -r
magic (which is fragile and something I don't want musl to depend on
since I know users will someday hit breakage and rightfully blame us
for using ld -r) to merge an asm source and C source. Or perhaps the
existing crti.s content could be moved to file-scope __asm__ included
in the C source file...that might be ok?

> > > My understanding of requirements for libc that exposes ssp support:
> > > - __stack_chk_fail is implemented as a default symbol
> > > - __stack_chk_fail_local is implemented as a local symbol to avoid PLT.
> > >   (Why is it important? To avoid use of potentially already broken stack?)
> > 
> > Because performance cost of -fstack-protector would go from 1-2% up to
> > 5-10% on i386 and other archs where PLT contract requires a GOT
> > register, since loading the GOT register is expensive
> > (__x86.get_pc_thunk.* thunk itself is somewhat costly, and you throw
> > away one of only a small number of available registers, increasing
> > register pressure and hurting codegen).
> 
> On Power it is just the setting up itself that is costly (in the config
> where we have this _local thing).

I think it'd be the same. If a function otherwise has no reason to
access global data or calls though PLT, it can avoid the cost of
finding the GOT and spending a fixed register on it. But possibility
of having to call __stack_chk_fail makes *every* (stack-protected)
function need to be able to make calls thru PLT, and thus introduces
this cost to every function.

> > Absolutely not. libssp is unsafe and creates new vulns/attack surface
> > by doing introspective stuff after the process is already *known to
> > be* in a compromised state. It should never be used. musl's
> > __stack_chk_fail is safe and terminates immediately.
> 
> Some implementations even print strings from the stack, it can be worse ;-)

:-)

> > Ideally, though, GCC would just emit the termination inline (or at
> > least have an option to do so) rather than calling __stack_chk_fail or
> > the local version. This would additionally harden against the case
> > where the GOT is compromised.
> 
> Yeah, but how to terminate is system-specific, it's much easier to punt
> this job to the libc to do ;-)

My ideas was __builtin_trap, although a slightly more hardened version
(that might make users unhappy? :) is inlining a syscall to
sigprocmask to mask SIGILL/SIGSEGV before the trapping instruction so
that termination occurs regardless of whether there's a signal handler
installed.

> Open a GCC PR for this please?

Filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93509

Rich

Re: [musl] musl, glibc and ideal place for __stack_chk_fail_local

[Segher Boessenkool]

On Thu, Jan 30, 2020 at 08:37:40AM -0500, Rich Felker wrote:
> On Thu, Jan 30, 2020 at 06:33:51AM -0600, Segher Boessenkool wrote:
> > On Sat, Jan 25, 2020 at 10:54:24AM -0500, Rich Felker wrote:
> > > > To support smash stack protection gcc emits __stack_chk_fail
> > > > calls on all targets. On top of that gcc emits __stack_chk_fail_local
> > > > calls at least on i386 and powerpc:
> > 
> > (Only on 32-bit -fPIC -msecure-plt, for Power).
> 
> Right, but musl only supports the secure-plt ABI.

Sure, it is the modern one.  Still only for 32-bit -fPIC for musl, too.

> > > There is a half-serious proposal to put it in crti.o which is always
> > > linked too, but that seems like an ugly hack to me...
> > 
> > Not *very* ugly, but it would be very effective, and no real downsides
> > to it (or do you see something?)
> 
> Well either the thunk has to be written in asm per-arch, or some ld -r
> magic (which is fragile and something I don't want musl to depend on
> since I know users will someday hit breakage and rightfully blame us
> for using ld -r) to merge an asm source and C source. Or perhaps the
> existing crti.s content could be moved to file-scope __asm__ included
> in the C source file...that might be ok?

At least for powerpc, the existing crti.s gets stuff inserted after (in
both functions), and then closed off by crtn.s -- not something you want
to do in C :-)

GCC can just say to also use extra crti files -- see STARTFILE_SPEC.
Many platforms do that already.

> > On Power it is just the setting up itself that is costly (in the config
> > where we have this _local thing).
> 
> I think it'd be the same.

We don't have a shortage of usable registers, that's what I was getting at.
All the other arguments are similar, sure.

> > > Absolutely not. libssp is unsafe and creates new vulns/attack surface
> > > by doing introspective stuff after the process is already *known to
> > > be* in a compromised state. It should never be used. musl's
> > > __stack_chk_fail is safe and terminates immediately.
> > 
> > Some implementations even print strings from the stack, it can be worse ;-)
> 
> :-)

It wasn't a joke, unfortunately.

> > > Ideally, though, GCC would just emit the termination inline (or at
> > > least have an option to do so) rather than calling __stack_chk_fail or
> > > the local version. This would additionally harden against the case
> > > where the GOT is compromised.
> > 
> > Yeah, but how to terminate is system-specific, it's much easier to punt
> > this job to the libc to do ;-)
> 
> My ideas was __builtin_trap, although a slightly more hardened version
> (that might make users unhappy? :) is inlining a syscall to
> sigprocmask to mask SIGILL/SIGSEGV before the trapping instruction so
> that termination occurs regardless of whether there's a signal handler
> installed.

I think we should make this a separate RTL pattern?  Or a (noreturn)
libgcc function?  Anyway, let's talk in the PR :-)

> > Open a GCC PR for this please?
> 
> Filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93509

Thanks!


Segher