On Sun, Jun 25, 2017 at 8:13 PM, Andrew Pinski <pins...@gmail.com> wrote: > On Sun, Jun 25, 2017 at 12:08 PM, Yuri Gribov <tetra2...@gmail.com> wrote: >> Hi all, >> >> Libgcc unwinder currently does not do any verification of pointers >> which it chases on stack. In practice this not so rarely causes >> segfaults when unwinding on corrupted stacks (e.g. when when trying to >> print diagnostic on >> fatal error) [1]. Ironically this usually happens in error reporting >> code which puzzles users. >> >> I've attached one motivating example - with current libgcc it will >> abort inside unwinder when trying to access invalid address >> (0x0a0a0a0a). >> >> There is an old request to provide a safer version of >> _Unwind_Backtrace (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67336) >> that would check pointer validity prior to dereferencing. I've >> attached a draft patch to see if this approach is viable and hopefully >> get some advice. >> >> The change is rather straightforward: I add a new >> _Unwind_Backtrace_Checked which checks return value of msync on every >> potentially unsafe access (libunwind does something like this as well, >> although in a very incomplete manner). >> To avoid paying for syscalls too often, I cache the last checked >> memory page. Potentially parsing /proc/$$/maps would allow for much >> faster verification but I didn't bother too much as new APIs are >> intended for reporting fatal errors where speed isn't an issue. >> >> The code is only implemented for DW2 unwinder (probably used on most >> targets). >> >> So my questions now are: >> 1) Would this feature considered useful i.e. will it be accepted for >> trunk once implementation is polished/tested? >> 2) Should I strive to implement it for all possible targets or DW2 >> would do for now? I don't have easy access to other platforms (ARM, >> C6x, etc.) so this may delay implementation. >> 3) Any suggestions/comments on this attached draft implementation? >> E.g. alternative syscalls to use (Andrew suggested futex), how many >> verified addresses to cache, whether I should verify unwind table >> accesses in addition to stack accesses, etc. > > The version script should be using GCC_8.0.0 since 6.2.0 has already > shipped months ago. > Also all patches should be submitted against the trunk and not a > released version.
Well, that's an RFC so I thought maybe original patch might be enough for general ok/not ok decision... -Y
