| 355080 |
25-Nov-2019 |
kevans |
MFC r354669, r354672, r354689: move __stack_chk_guard constructor
r354669:
ssp: add a priority to the __stack_chk_guard constructor
First, this commit is a NOP on GCC <= 4.x; this decidedly doesn't work
cleanly on GCC 4.2, and it will be gone soon anyways so I chose not to dump
time into figuring out if there's a way to make it work. xtoolchain-gcc,
clocking in as GCC6, can cope with it just fine and later versions are also
generally ok with the syntax. I suspect very few users are running GCC4.2
built worlds and also experiencing potential fallout from the status quo.
For dynamically linked applications, this change also means very little.
rtld will run libc ctors before most others, so the situation is
approximately a NOP for these as well.
The real cause for this change is statically linked applications doing
almost questionable things in their constructors. qemu-user-static, for
instance, creates a thread in a global constructor for their async rcu
callbacks. In general, this works in other places-
- On OpenBSD, __stack_chk_guard is stored in an .openbsd.randomdata section
that's initialized by the kernel in the static case, or ld.so in the
dynamic case
- On Linux, __stack_chk_guard is apparently stored in TLS and such a problem
is circumvented there because the value is presumed stable in the new
thread.
On FreeBSD, the rcu thread creation ctor and __guard_setup are both unmarked
priority. qemu-user-static spins up the rcu thread prior to __guard_setup
which starts making function calls- some of these are sprinkled with the
canary. In the middle of one of these functions, __guard_setup is invoked in
the main thread and __stack_chk_guard changes- qemu-user-static is promptly
terminated for an SSP violation that didn't actually happen.
This is not an all-too-common problem. We circumvent it here by giving the
__stack_chk_guard constructor a solid priority. 200 was chosen because that
gives static applications ample range (down to 101) for working around it
if they really need to. I suspect most applications will "just work" as
expected- the default/non-prioritized flavor of __constructor__ functions
run last, and the canary is generally not expected to change as of this
point at the very least.
This took approximately three weeks of spare time debugging to pin down.
r354672:
ssp: rework the logic to use priority=200 on clang builds
The preproc logic was added at the last minute to appease GCC 4.2, and
kevans@ did clearly not go back and double-check that the logic worked out
for clang builds to use the new variant.
It turns out that clang defines __GNUC__ == 4. Flip it around and check
__clang__ as well, leaving a note to remove it later.
r354689:
ssp: further refine the conditional used for constructor priority
__has_attribute(__constructor__) is a better test for clang than
defined(__clang__). Switch to it instead.
While we're already here and touching it, pfg@ nailed down when GCC actually
introduced the priority argument -- 4.3. Use that instead of our
hammer-guess of GCC >= 5 for the sake of correctness.
PR: 241905 |