[PATCH 0/1] MR3708: loader: Don't return from __stack_chk_fail() stub (which GCC expects to be noreturn) in preloader.
GCC expects __stack_chk_fail() to be noreturn[^1]. Returning from __stack_chk_fail() can thus lead to subtle failures. When crashing, use a volatile NULL pointer dereference. Ideally we would like to "abort()" here, but doing so would require two more syscall definitions just for a cold function. After all, the function isn't even used at all if -fno-stack-protector is specified. Also, don't say "stack smashing" (unlike glibc). The preloader currently initializes the stack canary value to a fixed value (0), which serves little value in protecting against actual buffer overrun attacks. [^1]: https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gccint/Stack-Smashing-Protection.ht... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3708
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> GCC expects __stack_chk_fail() to be noreturn[1]. Returning from __stack_chk_fail() can thus lead to subtle failures. When crashing, use a volatile NULL pointer dereference. Ideally we would like to "abort()" here, but doing so would require two more syscall definitions just for a cold function. After all, the function isn't even used at all if -fno-stack-protector is specified. Also, don't say "stack smashing" (unlike glibc). The preloader currently initializes the stack canary value to a fixed value (0), which serves little value in protecting against actual buffer overrun attacks. [1]: https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gccint/Stack-Smashing-Protection.ht... --- loader/preloader.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/loader/preloader.c b/loader/preloader.c index 635e85ee7cb..64db9205ac0 100644 --- a/loader/preloader.c +++ b/loader/preloader.c @@ -175,11 +175,6 @@ struct wld_auxv */ void __bb_init_func(void) { return; } -/* similar to the above but for -fstack-protector */ -void *__stack_chk_guard = 0; -void __stack_chk_fail_local(void) { return; } -void __stack_chk_fail(void) { return; } - #ifdef __i386__ /* data for setting up the glibc-style thread-local storage in %gs */ @@ -759,6 +754,34 @@ static __attribute__((noreturn,format(printf,1,2))) void fatal_error(const char wld_exit(1); } +/* + * The __stack_chk_fail is a no-return function only called when file is + * compiled with gcc flags "-fstack-protector". This function is normally + * provided by libc's startup files, but since we build the preloader with + * "-nostartfiles -nodefaultlibs", we have to provide our own version, + * otherwise linker fails. + */ +unsigned long __stack_chk_guard = 0; + +void __attribute__((noreturn)) __stack_chk_fail(void) +{ + const char message[] = "preloader: stack overrun detected, crashing\n"; + + /* Avoid using non-syscall functions that can re-enter this function */ + wld_write(2, message, sizeof(message) - 1); + + /* Deliberate induce crash and possibly dump core */ + *(volatile char *)0; + + /* Last resort if the zero page turns out to be actually readable */ + wld_exit(1); +} + +void __attribute__((noreturn)) __stack_chk_fail_local(void) +{ + __stack_chk_fail(); +} + #ifdef DUMP_AUX_INFO /* * Dump interesting bits of the ELF auxv_t structure that is passed -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3708
participants (2)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)