Skip to content

Commit

Permalink
LibC: Move stack canary initialization before the global constructors
Browse files Browse the repository at this point in the history
Once again, QEMU creates threads while running its constructors, which
is a recipe for disaster if we switch out the stack guard while that is
already running in the background.

To solve that, move initialization to our LibC initialization stage,
which is before any actual external initialization code runs.
  • Loading branch information
timschumi authored and bgianfo committed Jul 8, 2022
1 parent cf0ad37 commit b9f7966
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 16 deletions.
16 changes: 0 additions & 16 deletions Userland/Libraries/LibC/crt0.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,6 @@ NAKED void _start(int, char**, char**)

int _entry(int argc, char** argv, char** env)
{
size_t original_stack_chk = __stack_chk_guard;

// We can't directly overwrite __stack_chk_guard using arc4random_buf,
// as it doesn't know that the stack canary changed and it would instantly
// cause a stack protector failure when returning.
size_t new_stack_chk = 0;
arc4random_buf(&new_stack_chk, sizeof(new_stack_chk));

if (new_stack_chk != 0)
__stack_chk_guard = new_stack_chk;

environ = env;
__environ_is_malloced = false;
__begin_atexit_locking();
Expand All @@ -55,11 +44,6 @@ int _entry(int argc, char** argv, char** env)

exit(status);

// We should never get here, but if we ever do, make sure to
// restore the stack guard to the value we entered _start with.
// Then we won't trigger the stack canary check on the way out.
__stack_chk_guard = original_stack_chk;

return 20150614;
}
}
Expand Down
6 changes: 6 additions & 0 deletions Userland/Libraries/LibELF/DynamicLinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ static void initialize_libc(DynamicObject& libc)
VERIFY(res.has_value());
*((char***)res.value().address.as_ptr()) = s_envp;

// __stack_chk_guard should be initialized before anything significant (read: global constructors) is running.
// This is not done in __libc_init, as we definitely have to return from that, and it might affect Loader as well.
res = libc.lookup_symbol("__stack_chk_guard"sv);
VERIFY(res.has_value());
arc4random_buf(res.value().address.as_ptr(), sizeof(size_t));

res = libc.lookup_symbol("__environ_is_malloced"sv);
VERIFY(res.has_value());
*((bool*)res.value().address.as_ptr()) = false;
Expand Down

0 comments on commit b9f7966

Please sign in to comment.