Evaluate stack guard implementation(s) #8666
Comments
|
The CONT and StackThunk guards and AIUI The CONT and StackThunk guards check for total stack overflow, after the fact. Too many levels of calls or too large locals. (i.e. every funciton was well behaved, but they needed, say, 5K of stack) GCC never knows the real total stack limits, that's more of the OS's job to handle. One idea toyed with before (there should be a PR or discussion w/a code sample in fact) was to set the HW data breakpoint to the last 16 (or other power-of-two) bytes of the stack. With that you would get the Cont/Thunk stack checker behavior, instantly. A pie in the sky idea would be to adjust the GCC xtensa machine description to check any stack allocations at runtime against some well-known "current stack limits". Pretty massive performance penalty and it would not, obviously, do anything with the blob code. |
Just curious. Edit: |
|
Every variant of stack smashing protection seem to utilize a canary value that is either nearby the stack memory or in case of protector just added as an additional stack variable that is created on stack immediately as function is called. I think I misunderstood the way boundaries are set, so it does not seem that CONT could interact with the protector code that well just by checking that nearby memory is untouched :/ The repainting still seems suspect, don't either guards or paint value need to change so we don't mistake structure with the array itself when both values are equal? The mentioned code that does HW breakpoint is #5158 Machine-definition way seems to be related to Option adds extra code at the start of the func and checks whether (or any other GCC instrumentation does not seem to help; even |
|
True, but isn't it enough for functions that allocate some stack and try to operate it in the 'forbidden' region? Just another layer on top of existing debugging options.
Coherent error reporting, at the very least. Like in the mentioned error sanitizer example or the way stack-check does it. The point is not to recover, but to reduce the time from getting the error and noticing 'oops this is a stack overflow / smashing' (which btw is also mis-named in StackThunk) |
Be careful that memory regions can often be accessed discontinuously with stride of tens (or more) of bytes.
And that is all, I guess. It is much better to be interrupted by the stack guard in a potentially problematic place than by the WDT in a completely unrelated one. |
|
Just one thing you might want to look into, just to be sure about whether the current "stack painting" is working as it should. |
Hmm, didn't GCC have a counterpart to SecureZeroMemory() in VC++? |
In GCC, we can use extended asm to force dependency injection: Now GCC recognizes that all the elements of |
|
I don't think we have that issue with 'repaints' themselves? It is a separate func call, plus code does not seem to optimize to memset and simply uses loop. Notably, we have Comparing some code, FreeBSD does not employ the any barriers glibc source does Will update with the stack errors reporting, so cont and bssl attach our postmortem stack pointer error via this, plus rename to separate smash from overflow Arduino/cores/esp8266/core_esp8266_postmortem.cpp Lines 156 to 157 in 313b3c0 btw I noticed stack watchpoint / breakpoint is also implemented in esp-idf for tasks stack. Each time they switch, this check is set up |
Wire everything that relies on stack smashing detection to call `__stack_chk_fail()` (aka what libssp / ssp / stack-protector uses) Expose it in our debugging header Rename overflow -> smashing, as these are different things we are trying to detect (meaning, that we check for things writing there, not some kind of `alloca` issue or the way `-fstack-check` would have worked) ref. esp8266#8666 `-fstack-protector` continues to work as it always did CONT replaces `abort()`, also moves its check to the loop wrapper to avoid dumping otherwise useless SYS context memory StackThunk replaces a similar `abort()` call
Wire everything that relies on stack smashing detection to call `__stack_chk_fail()` (aka what libssp / ssp / stack-protector uses) Expose it in our debugging header Rename overflow -> smashing, as these are different things we are trying to detect (meaning, that we check for things writing there, not some kind of `alloca` issue or the way `-fstack-check` would have worked) ref. #8666 `-fstack-protector` continues to work as it always did CONT replaces `abort()`, also moves its check to the loop wrapper to avoid dumping otherwise useless SYS context memory StackThunk replaces a similar `abort()` call
Wire everything that relies on stack smashing detection to call `__stack_chk_fail()` (aka what libssp / ssp / stack-protector uses) Expose it in our debugging header Rename overflow -> smashing, as these are different things we are trying to detect (meaning, that we check for things writing there, not some kind of `alloca` issue or the way `-fstack-check` would have worked) ref. esp8266#8666 `-fstack-protector` continues to work as it always did CONT replaces `abort()`, also moves its check to the loop wrapper to avoid dumping otherwise useless SYS context memory StackThunk replaces a similar `abort()` call
Right now we have at least three ways to ensure stack-smash does not happen.
cont_...functions related to checks and 're-painting'-fstack-protectorthat instruments functions (also__attribute__((stack_protect))to manually inject such checks) with guard variables and a special__stack_chk_{guard,fail()}; ref. https://gcc.gnu.org/onlinedocs/gccint/Stack-Smashing-Protection.html and https://github.com/esp8266/Arduino/blob/master/cores/esp8266/core_esp8266_postmortem.cpp implementationsstack_thunk_fatal_overflow()and main implementation in https://github.com/esp8266/Arduino/blob/master/cores/esp8266/StackThunk.h)Could GCC implementation supplement both StackThunk and CONT? Or, replace it? (see
__attribute__above)Should CONT checks randomize its guard value to separate stack contents themselves from structure guard members?
Could we add something like address sanitizer that verifies that we don't go over the stack boundaries, not just protect us from writing things over one specific u32 value?
w
The text was updated successfully, but these errors were encountered: