This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: --enable-stack-protector for glibc, v7


On 4 Jul 2016, Florian Weimer said:

> On 06/27/2016 05:27 PM, Nix wrote:
>>> (It might also help to avoid dropping stack protection from IFUNCs,
>>> but since IFUNCs can do so little, that should not matter.)
>>
>> I don't think we can avoid that.  In the dynamic-linking case, the
>> canary is initialized a long, long time after ifunc resolvers get
>> applied and ifuncs start getting called, and I don't want to start
>> messing with ld.so in this patch series :)
>
> But this affects not just IFUNC resolvers in glibc, but also external
> IFUNC resolvers in static libraries. They might have been compiled
> with -fstack-protector-all, and require a TCB.

And they'll crash before this patch and will crash afterwards. There is
no change (as you note below). This is probably more important now that
things like function multiversioning are making heavier use of ifuncs
(though I guess that, being part of GCC itself, the function
multiversioning takes care not to stack-protect the resolvers even if
the file is compiled with -fstack-protector-all? If not, that would seem
to be a bug in GCC.)

> The issue here is not that the stack guard has not been initialized,
> it's that the TCB address has not been set. And as a result of that,
> any TLS is just invalid. This would also affect error reporting
> through errno.

Agreed (though any ifunc resolvers using errno are in for a hard, hard
time).

> If we can land Stefan Liebler's IFUNC cleanup, then disabling
> stack-protector for IFUNC resolvers will be a very simple patch. So we
> could start with that approach,

You are of course quite welcome to do that at merge time :) all the
ifunc stuff is in one patch here so is easy for you to drop and replace
with something better. I am not emotionally attached to the bitty
touch-every-resolver approach I'm using now, indeed I'd prefer something
better since the whole thrust of this patch series has been to avoid
having to do anything to every class of any function ('every function in
ld.so' for instance) because the result would be more or less
unmaintainable.

>                                 and revisit this topic later. As you
> pointed out, we are not changing the IFUNC resolver calling order with
> regards to TCB setup, so lack of support for stack-protected IFUNC
> resolvers is not a regression.

Agreed, but that does mean that we need to prevent the things from being
stack-protected!

(fwiw, all tests passed against my patch series with every review
request incorporated bar this one, including the dropping of the
open-coding-strcpy() patch in favour of no-stack-protecting memcpy() et
al: want a reposting? This *does* include the ifuncing up of stuff, for
now, because we do need to do this somehow or all the tests explode in
fire and tears. But those are self-contained and can be replaced with a
better approach easily enough.)

-- 
NULL && (void)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]