Page MenuHomeFreeBSD

libc: gen: add a test for rtld underflowing our posix_spawn thread
ClosedPublic

Authored by kevans on Mon, Jun 29, 8:48 PM.
Tags
None
Referenced Files
F161695444: D57954.diff
Mon, Jul 6, 12:48 AM
Unknown Object (File)
Sun, Jul 5, 6:51 AM
Unknown Object (File)
Sat, Jul 4, 5:45 AM
Unknown Object (File)
Sat, Jul 4, 5:32 AM
Unknown Object (File)
Fri, Jul 3, 3:37 PM
Unknown Object (File)
Fri, Jul 3, 3:26 PM
Unknown Object (File)
Fri, Jul 3, 1:25 PM
Unknown Object (File)
Fri, Jul 3, 5:31 AM
Subscribers

Details

Summary

This is a distillation of the environment described in the PR, using
a dummy shlib and mapping it repeatedly. In isolation this test is
a little useless, but when run under ASAN we'll quickly surface the
problem described in the PR.

Future work might want to add another page to the posix_spawn allocation
and perhaps try to add a guard page below it, but it's not clear that
it's worth the effort. We have a ~4k allocation at the point that rtld
is resolving the likely candidate and only a little over 1k consumed at
that point. execvPe_prog() will allocate one pointer per-arg in the
case of an ENOEXEC fallback, but the subsequent memcpy() isn't likely to
take us on a trip through rtld so we should still have most of our 3k
stack for this.

PR: 295991

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/libc/tests/gen/posix_spawn_test.c
232

An earlier version of this change also tried to create unique SOVERSIONs for every loaded object, but it seems to be sufficient to just have a different st_dev/st_ino for each one. I retained this assertion just in case something goes wrong, though.

If NUM_DSO is large enough, I believe it should trips without any sanitizer.

lib/libc/tests/gen/posix_spawn_test.c
199

why RDWR?

216

why PROT_WRITE?

In D57954#1328617, @kib wrote:

If NUM_DSO is large enough, I believe it should trips without any sanitizer.

Right, it will still underflow, but the hard part is detecting that so that you can trip an assertion. I think we'd probably want a custom allocator to be able to influence where the rfork_thread stack is laid out, so that we can guarantee where the damage will occur and spot it. Otherwise, I think we have to spray a bunch of patterned buffers onto the heap and posix_spawn a bunch, then verify that all of our buffers are still intact?

kevans marked 2 inline comments as done.

Downgrade our shlib to RDONLY and MAP_SHARED, now that we're not trying to do
a unique SOVERSION for every fdlopen().

In D57954#1328617, @kib wrote:

If NUM_DSO is large enough, I believe it should trips without any sanitizer.

Right, it will still underflow, but the hard part is detecting that so that you can trip an assertion. I think we'd probably want a custom allocator to be able to influence where the rfork_thread stack is laid out, so that we can guarantee where the damage will occur and spot it. Otherwise, I think we have to spray a bunch of patterned buffers onto the heap and posix_spawn a bunch, then verify that all of our buffers are still intact?

If the test program segfaults, wouldn't kyua declare the test failing?
Or you could fork, detect the child exit due to a signal, and declare test failing.

In D57954#1328625, @kib wrote:
In D57954#1328617, @kib wrote:

If NUM_DSO is large enough, I believe it should trips without any sanitizer.

Right, it will still underflow, but the hard part is detecting that so that you can trip an assertion. I think we'd probably want a custom allocator to be able to influence where the rfork_thread stack is laid out, so that we can guarantee where the damage will occur and spot it. Otherwise, I think we have to spray a bunch of patterned buffers onto the heap and posix_spawn a bunch, then verify that all of our buffers are still intact?

If the test program segfaults, wouldn't kyua declare the test failing?
Or you could fork, detect the child exit due to a signal, and declare test failing.

Right, but what drives the test program segfaulting? Without ASAN, we have to actually damage something important enough to cause some fallout. For reasons that I haven't debugged at the moment, we're not actually catching a SIGSEGV/SIGBUS in this test case -- I'm guessing the stack is placed adjacent to something valid, but the damage is apparently not problematic. If I bump NUM_DSO up to something stupid like 16k, it *still* doesn't actually catch a signal without ASAN.

Given that it's still not even at absurdly large numbers, I wonder if it's coincidentally just wiping out the mapped contents of these DSOs. Maybe I should re-design this to then call into every single one of them after the posix_spawnp.

Given that it's still not even at absurdly large numbers, I wonder if it's coincidentally just wiping out the mapped contents of these DSOs. Maybe I should re-design this to then call into every single one of them after the posix_spawnp.

Try to add a guard page to the posix_spawn() stack instead. I think that this is very good idea anyway.

In D57954#1328630, @kib wrote:

Given that it's still not even at absurdly large numbers, I wonder if it's coincidentally just wiping out the mapped contents of these DSOs. Maybe I should re-design this to then call into every single one of them after the posix_spawnp.

Try to add a guard page to the posix_spawn() stack instead. I think that this is very good idea anyway.

D57955

With the guard page landed, I'm going to rework this a little and likely try posix_spawnp at each fdlopen (or every few) up until 256 to exercise both the old regression and the new transition point from alloca -> heap after your fix.

Increase NUM_DSO to a number that reliably hits the guard page without ASAN.

I realized after my last comment that we only get one shot to trigger this,
since we can't really unwind the results of rtld_bind. Overall execution remains
the same.

This revision is now accepted and ready to land.Tue, Jun 30, 8:08 AM