Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 03.08.2017 18:04, Florian Weimer wrote: * Yuri Gribov: I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html What about the remaining TODOs? Yes, need to be fixed. Note that patch is an RFC so I didn't want to polish it until I get enough feedback from others. + if (__builtin_expect (msync ((void *)page, len, MS_ASYNC), 0)) +return 0; Is there any reason not to probe using mincore? It won't trigger writeback. Nice, for some reason I didn't realize it provides necessary info. -Y
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 08/03/2017 11:03 AM, Florian Weimer wrote: > * Jeff Law: > >>> If speed is not a concern, but reliability is, call fork (the system >>> call, not glibc's wrapper which calls fork handlers) and do the work >>> in a single-threaded copy of the process. There, you can set up >>> signal handlers as you see fit, and the VM layout won't change >>> unexpectedly. > >> Speed is one of the concerns. > > I thought this was debugging-only, optional unwinder behavior. My understanding was this was in the EH path as well. If it's outside the EH path, then I don't care about the cost of msync and the like :-) > >> Essentially we're pondering an efficient >> mechanism to know if a given code address is valid so that we know if we >> should even try to unwind through a particular stack frame. > > Efficient code address validation is possible using data structures > maintained by ld.so. If those data structures are mostly read-only, > they cannot be corrupted, so pointers and offsets in them don't have > to be validated before derefencing them. Understood. > > But the main problem is the frame pointer chain, and the current lack > of registration for stacks. The program can just malloc some area, > switch the SP, and start running from there. Some libraries do > exactly that. And without knowledge of the bottom address of the > stack, it is impossible to verify that the frame pointer chain goes > somewhere off the stack. Yea. > >> One could probably claim that in the EH path a call to msync shouldn't >> be a concern. I'm skeptical these days, but could be convinced that >> it's tolerable. > > Quite a few applications are sensitive to EH performance. The current > freshness check for the cache is very inefficient, which is a > significant problem for some users. If we can push out the flag check > the top level (essentially compiling the unwinder twice, with and > without checking), I don't think this would cause significant > additional performance issues. Yea. I went back and reviewed the internal BZ. It's actually just the print-a-backtrace thing rather than EH unwinding. SO yea, if the print-a-backtrace path and EH path both use bits, we could just compile them twice, once with checking once without respectively. Jeff
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
* Yuri Gribov: > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html What about the remaining TODOs? > + if (__builtin_expect (msync ((void *)page, len, MS_ASYNC), 0)) > +return 0; Is there any reason not to probe using mincore? It won't trigger writeback.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
* Jeff Law: >> If speed is not a concern, but reliability is, call fork (the system >> call, not glibc's wrapper which calls fork handlers) and do the work >> in a single-threaded copy of the process. There, you can set up >> signal handlers as you see fit, and the VM layout won't change >> unexpectedly. > Speed is one of the concerns. I thought this was debugging-only, optional unwinder behavior. > Essentially we're pondering an efficient > mechanism to know if a given code address is valid so that we know if we > should even try to unwind through a particular stack frame. Efficient code address validation is possible using data structures maintained by ld.so. If those data structures are mostly read-only, they cannot be corrupted, so pointers and offsets in them don't have to be validated before derefencing them. But the main problem is the frame pointer chain, and the current lack of registration for stacks. The program can just malloc some area, switch the SP, and start running from there. Some libraries do exactly that. And without knowledge of the bottom address of the stack, it is impossible to verify that the frame pointer chain goes somewhere off the stack. > One could probably claim that in the EH path a call to msync shouldn't > be a concern. I'm skeptical these days, but could be convinced that > it's tolerable. Quite a few applications are sensitive to EH performance. The current freshness check for the cache is very inefficient, which is a significant problem for some users. If we can push out the flag check the top level (essentially compiling the unwinder twice, with and without checking), I don't think this would cause significant additional performance issues.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 03, 2017 at 09:48:47AM -0600, Jeff Law wrote: > > If speed is not a concern, but reliability is, call fork (the system > > call, not glibc's wrapper which calls fork handlers) and do the work > > in a single-threaded copy of the process. There, you can set up > > signal handlers as you see fit, and the VM layout won't change > > unexpectedly. > Speed is one of the concerns. Essentially we're pondering an efficient > mechanism to know if a given code address is valid so that we know if we > should even try to unwind through a particular stack frame. > > One could probably claim that in the EH path a call to msync shouldn't > be a concern. I'm skeptical these days, but could be convinced that > it's tolerable. There are many projects that abuse exception handling for normal control flow and where the EH speed is very important. While the msync calls were not performed during normal EH or normal backtrace in the posted patch, just in sections guarded with __builtin_expect on some variable, it still makes worse RA because the compiler has to store and load all the state it could have in call clobbered registers across that spot. Jakub
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 4:28 PM, H.J. Lu wrote: > On Thu, Aug 3, 2017 at 8:25 AM, Jeff Law wrote: >> On 08/03/2017 08:17 AM, Yury Gribov wrote: >>> On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: >> On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov >> wrote: >>> On 02.08.2017 23:04, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: > > On 02.08.2017 21:48, H.J. Lu wrote: >> >> >> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >> >> wrote: >>> >>> >>> On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > > > > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> >> >> >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> >>> >>> >>> I've rebased the previous patch to trunk per Andrew's >>> suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> >> >> >> Is his stuff used for exception handling? If so, doesn't that >> make >> the >> performance a significant concern (ie that msync call?) > > > > > For the performance issue, I wonder if it wouldn't be better to > just > compile the unwinder twice, basically include everything needed > for > the > verification backtrace in a single *.c file with special defines, > and > not export anything from that *.o file except the single > entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. > For the concurrent calls of other threads unmapping stacks of > running > threads (that is where most of the verification goes against), I'm > afraid > that is simply non-solveable, even if you don't cache anything, it > will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. >>> >>> >>> >>> >>> All, >>> >>> First of all, thanks for the feedback. This is indeed not a 100% >>> robust >>> solution, just something to allow tools like Asan to produce more >>> sane >>> backtraces on crash (currently when stack is corrupted they would >>> crash >>> in >>> the unwinder without printing at least partial backtrace). >>> >> >> FWIW, glibc 2.26 is changed to abort as soon as possible when stack >> is >> corrupted: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >> >> There is very little point to unwind stack when stack is corrupted. > > > > I'd argue that situation with __stack_chk_fail is very special. It's > used > in production code where you'd want to halt as soon as corruption is > detected to prevent further damage so even printing message is an > additional > risk. Debugging tools (e.g. sanitizers) are different and would > prefer > to > print as many survived frames as possible. > But stack unwinder in libgcc is primarily for production use, not for debugging. >>> >>> >>> I can see it used in different projects to print backtraces on error >>> (bind9, >>> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder >>> and >>> seems to be meant primarily for debugging (at least many projects use >>> it for >>> this purpose: >>> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). >>> Same for >>> libbacktrace which, as its README mentions, is meant to "print a >>> detailed >>> backtrace when an error occurs or to gather detailed profiling >>> information".
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 08/02/2017 03:36 PM, Florian Weimer wrote: > * Jeff Law: > >> Something like setup a signal handler when we first start unwinding that >> flags the error and tear it down when we're done unwinding?Obviously >> we can't do setup/tear down each time for each address. Anyway, just >> thinking outloud here... > > Linux doesn't have per-thread signal handlers, so this doesn't work > reliably. Sigh. There's probably a multitude of reasons why this wouldn't work :-) > > If speed is not a concern, but reliability is, call fork (the system > call, not glibc's wrapper which calls fork handlers) and do the work > in a single-threaded copy of the process. There, you can set up > signal handlers as you see fit, and the VM layout won't change > unexpectedly. Speed is one of the concerns. Essentially we're pondering an efficient mechanism to know if a given code address is valid so that we know if we should even try to unwind through a particular stack frame. One could probably claim that in the EH path a call to msync shouldn't be a concern. I'm skeptical these days, but could be convinced that it's tolerable. > > To harden unwinding against corrupted tables or table locations, we'd > have to change ld.so to make all critical data read-only after loading > and remove the unwinder caches (with more help from ld.so instead). > It would make sense to move the unwinder implementation into ld.so. > With proper hardening, corrupted stacks would not be able to cause > crashes anymore, either. Yea. I'm not sure if we're ready to suggest that kind of change yet, but that may be the long term direction. jeff
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 08/03/2017 06:52 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: >> On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov >> wrote: >>> On 02.08.2017 23:04, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: > > On 02.08.2017 21:48, H.J. Lu wrote: >> >> >> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >> >> wrote: >>> >>> >>> On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > > > > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> >> >> >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> >>> >>> >>> I've rebased the previous patch to trunk per Andrew's suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> >> >> >> Is his stuff used for exception handling? If so, doesn't that make >> the >> performance a significant concern (ie that msync call?) > > > > > For the performance issue, I wonder if it wouldn't be better to just > compile the unwinder twice, basically include everything needed for > the > verification backtrace in a single *.c file with special defines, and > not export anything from that *.o file except the single entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. > For the concurrent calls of other threads unmapping stacks of running > threads (that is where most of the verification goes against), I'm > afraid > that is simply non-solveable, even if you don't cache anything, it > will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. >>> >>> >>> >>> >>> All, >>> >>> First of all, thanks for the feedback. This is indeed not a 100% >>> robust >>> solution, just something to allow tools like Asan to produce more sane >>> backtraces on crash (currently when stack is corrupted they would crash >>> in >>> the unwinder without printing at least partial backtrace). >>> >> >> FWIW, glibc 2.26 is changed to abort as soon as possible when stack is >> corrupted: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >> >> There is very little point to unwind stack when stack is corrupted. > > > > I'd argue that situation with __stack_chk_fail is very special. It's > used > in production code where you'd want to halt as soon as corruption is > detected to prevent further damage so even printing message is an > additional > risk. Debugging tools (e.g. sanitizers) are different and would prefer > to > print as many survived frames as possible. > But stack unwinder in libgcc is primarily for production use, not for debugging. >>> >>> >>> I can see it used in different projects to print backtraces on error (bind9, >>> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and >>> seems to be meant primarily for debugging (at least many projects use it for >>> this purpose: >>> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for >>> libbacktrace which, as its README mentions, is meant to "print a detailed >>> backtrace when an error occurs or to gather detailed profiling information". >> >> But it shouldn't be performed on a corrupted stack. When a stack is >> corrupted, there is just no way to reliably unwind it. > > Why not? Attached patch shows that it's possible (up to a point of > corruption of course). Right. We should unwind up to the point where we detect something amiss, then stop. And that's what I'm most interested in here -- detecting of the invalid frame data. THe problem I see is doing it efficiently enough. jeff
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 8:25 AM, Jeff Law wrote: > On 08/03/2017 08:17 AM, Yury Gribov wrote: >> On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: >>> On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov >>> wrote: On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: > On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov > wrote: >> On 02.08.2017 23:04, H.J. Lu wrote: >>> >>> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >>> >>> wrote: On 02.08.2017 21:48, H.J. Lu wrote: > > > On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov > > wrote: >> >> >> On 02.08.2017 20:02, Jeff Law wrote: >>> >>> >>> >>> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: > > > > On 07/17/2017 01:23 AM, Yuri Gribov wrote: >> >> >> >> I've rebased the previous patch to trunk per Andrew's suggestion. >> Original patch description/motivation/questions are in >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html > > > > Is his stuff used for exception handling? If so, doesn't that > make > the > performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. >>> >>> >>> >>> Yea. >>> >>> Given that the vast majority of the time the addresses are valid, I >>> wonder if we could take advantage of that property to keep the >>> overhead >>> lower in the most common cases. >>> For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. >>> >>> >>> >>> Absolutely -- I think solving this stuff 100% reliably without races >>> isn't possible. I think the question is can we make this >>> significantly >>> better. >> >> >> >> >> All, >> >> First of all, thanks for the feedback. This is indeed not a 100% >> robust >> solution, just something to allow tools like Asan to produce more >> sane >> backtraces on crash (currently when stack is corrupted they would >> crash >> in >> the unwinder without printing at least partial backtrace). >> > > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is > corrupted: > > https://sourceware.org/bugzilla/show_bug.cgi?id=21752 > https://sourceware.org/bugzilla/show_bug.cgi?id=12189 > > There is very little point to unwind stack when stack is corrupted. I'd argue that situation with __stack_chk_fail is very special. It's used in production code where you'd want to halt as soon as corruption is detected to prevent further damage so even printing message is an additional risk. Debugging tools (e.g. sanitizers) are different and would prefer to print as many survived frames as possible. >>> >>> But stack unwinder in libgcc is primarily for production use, not for >>> debugging. >> >> >> I can see it used in different projects to print backtraces on error >> (bind9, >> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder >> and >> seems to be meant primarily for debugging (at least many projects use it >> for >> this purpose: >> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same >> for >> libbacktrace which, as its README mentions, is meant to "print a detailed >> backtrace when an error occurs or to gather detailed profiling >> information". > > But it shouldn't be performed on a corrupted stack. When a stack is > corrupted, there is just no way to reliably unwind it. Why not? Attached patch shows that it's possible (up to a point of corruption of course). > It is
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 08/03/2017 08:17 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: >> On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov >> wrote: >>> On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov wrote: > On 02.08.2017 23:04, H.J. Lu wrote: >> >> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >> wrote: >>> >>> On 02.08.2017 21:48, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: > > > On 02.08.2017 20:02, Jeff Law wrote: >> >> >> >> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: >>> >>> >>> >>> On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: > > > > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) >>> >>> >>> >>> >>> For the performance issue, I wonder if it wouldn't be better to just >>> compile the unwinder twice, basically include everything needed for >>> the >>> verification backtrace in a single *.c file with special defines, >>> and >>> not export anything from that *.o file except the single entrypoint. >>> That would also handle the ABI problems. >> >> >> >> Yea. >> >> Given that the vast majority of the time the addresses are valid, I >> wonder if we could take advantage of that property to keep the >> overhead >> lower in the most common cases. >> >>> For the concurrent calls of other threads unmapping stacks of >>> running >>> threads (that is where most of the verification goes against), I'm >>> afraid >>> that is simply non-solveable, even if you don't cache anything, it >>> will >>> still be racy. >> >> >> >> Absolutely -- I think solving this stuff 100% reliably without races >> isn't possible. I think the question is can we make this >> significantly >> better. > > > > > All, > > First of all, thanks for the feedback. This is indeed not a 100% > robust > solution, just something to allow tools like Asan to produce more sane > backtraces on crash (currently when stack is corrupted they would > crash > in > the unwinder without printing at least partial backtrace). > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. >>> >>> >>> >>> I'd argue that situation with __stack_chk_fail is very special. It's >>> used >>> in production code where you'd want to halt as soon as corruption is >>> detected to prevent further damage so even printing message is an >>> additional >>> risk. Debugging tools (e.g. sanitizers) are different and would prefer >>> to >>> print as many survived frames as possible. >>> >> >> But stack unwinder in libgcc is primarily for production use, not for >> debugging. > > > I can see it used in different projects to print backtraces on error > (bind9, > SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder > and > seems to be meant primarily for debugging (at least many projects use it > for > this purpose: > https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same > for > libbacktrace which, as its README mentions, is meant to "print a detailed > backtrace when an error occurs or to gather detailed profiling > information". But it shouldn't be performed on a corrupted stack. When a stack is corrupted, there is just no way to reliably unwind it. >>> >>> Why not? Attached patch shows that it's possible (up to a point of >>> corruption of course). >>> It isn't a problem for a debugger which is designed to analyze corrupted program. >>> >>> Yes but forcing all applications that would like to print helpful >>> message on stack corruption to employ gdb sounds less efficient that >>> providing fail-safe
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 8:07 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 3:55 PM, H.J. Lu wrote: >> On Thu, Aug 3, 2017 at 7:17 AM, Yury Gribov >> wrote: >>> On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: >> On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov >> wrote: >>> On 02.08.2017 23:04, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: > > On 02.08.2017 21:48, H.J. Lu wrote: >> >> >> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >> >> wrote: >>> >>> >>> On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > > > > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> >> >> >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> >>> >>> >>> I've rebased the previous patch to trunk per Andrew's >>> suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> >> >> >> Is his stuff used for exception handling? If so, doesn't that >> make >> the >> performance a significant concern (ie that msync call?) > > > > > For the performance issue, I wonder if it wouldn't be better to > just > compile the unwinder twice, basically include everything needed > for > the > verification backtrace in a single *.c file with special defines, > and > not export anything from that *.o file except the single > entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. > For the concurrent calls of other threads unmapping stacks of > running > threads (that is where most of the verification goes against), I'm > afraid > that is simply non-solveable, even if you don't cache anything, it > will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. >>> >>> >>> >>> >>> All, >>> >>> First of all, thanks for the feedback. This is indeed not a 100% >>> robust >>> solution, just something to allow tools like Asan to produce more >>> sane >>> backtraces on crash (currently when stack is corrupted they would >>> crash >>> in >>> the unwinder without printing at least partial backtrace). >>> >> >> FWIW, glibc 2.26 is changed to abort as soon as possible when stack >> is >> corrupted: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >> >> There is very little point to unwind stack when stack is corrupted. > > > > I'd argue that situation with __stack_chk_fail is very special. It's > used > in production code where you'd want to halt as soon as corruption is > detected to prevent further damage so even printing message is an > additional > risk. Debugging tools (e.g. sanitizers) are different and would > prefer > to > print as many survived frames as possible. > But stack unwinder in libgcc is primarily for production use, not for debugging. >>> >>> >>> I can see it used in different projects to print backtraces on error >>> (bind9, >>> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder >>> and >>> seems to be meant primarily for debugging (at least many projects use >>> it for >>> this purpose: >>> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). >>> Same for >>> libbacktrace which, as its README mentions, is meant to "print a >>> detailed >>> backtrace when an error occurs or to gather detailed profiling >
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 3:55 PM, H.J. Lu wrote: > On Thu, Aug 3, 2017 at 7:17 AM, Yury Gribov > wrote: >> On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: >>> On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov >>> wrote: On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: > On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov > wrote: >> On 02.08.2017 23:04, H.J. Lu wrote: >>> >>> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >>> >>> wrote: On 02.08.2017 21:48, H.J. Lu wrote: > > > On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov > > wrote: >> >> >> On 02.08.2017 20:02, Jeff Law wrote: >>> >>> >>> >>> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: > > > > On 07/17/2017 01:23 AM, Yuri Gribov wrote: >> >> >> >> I've rebased the previous patch to trunk per Andrew's suggestion. >> Original patch description/motivation/questions are in >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html > > > > Is his stuff used for exception handling? If so, doesn't that > make > the > performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. >>> >>> >>> >>> Yea. >>> >>> Given that the vast majority of the time the addresses are valid, I >>> wonder if we could take advantage of that property to keep the >>> overhead >>> lower in the most common cases. >>> For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. >>> >>> >>> >>> Absolutely -- I think solving this stuff 100% reliably without races >>> isn't possible. I think the question is can we make this >>> significantly >>> better. >> >> >> >> >> All, >> >> First of all, thanks for the feedback. This is indeed not a 100% >> robust >> solution, just something to allow tools like Asan to produce more >> sane >> backtraces on crash (currently when stack is corrupted they would >> crash >> in >> the unwinder without printing at least partial backtrace). >> > > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is > corrupted: > > https://sourceware.org/bugzilla/show_bug.cgi?id=21752 > https://sourceware.org/bugzilla/show_bug.cgi?id=12189 > > There is very little point to unwind stack when stack is corrupted. I'd argue that situation with __stack_chk_fail is very special. It's used in production code where you'd want to halt as soon as corruption is detected to prevent further damage so even printing message is an additional risk. Debugging tools (e.g. sanitizers) are different and would prefer to print as many survived frames as possible. >>> >>> But stack unwinder in libgcc is primarily for production use, not for >>> debugging. >> >> >> I can see it used in different projects to print backtraces on error >> (bind9, >> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder >> and >> seems to be meant primarily for debugging (at least many projects use it >> for >> this purpose: >> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same >> for >> libbacktrace which, as its README mentions, is meant to "print a detailed >> backtrace when an error occurs or to gather detailed profiling >> information". > > But it shouldn't be performed on a corrupted stack. When a stack is > corrupted, there is just no way to reliably unwind it. Why not? Attached patch shows that it's possible (up to a point of corruption of course).
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 7:17 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: >> On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov >> wrote: >>> On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov wrote: > On 02.08.2017 23:04, H.J. Lu wrote: >> >> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >> wrote: >>> >>> On 02.08.2017 21:48, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: > > > On 02.08.2017 20:02, Jeff Law wrote: >> >> >> >> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: >>> >>> >>> >>> On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: > > > > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) >>> >>> >>> >>> >>> For the performance issue, I wonder if it wouldn't be better to just >>> compile the unwinder twice, basically include everything needed for >>> the >>> verification backtrace in a single *.c file with special defines, >>> and >>> not export anything from that *.o file except the single entrypoint. >>> That would also handle the ABI problems. >> >> >> >> Yea. >> >> Given that the vast majority of the time the addresses are valid, I >> wonder if we could take advantage of that property to keep the >> overhead >> lower in the most common cases. >> >>> For the concurrent calls of other threads unmapping stacks of >>> running >>> threads (that is where most of the verification goes against), I'm >>> afraid >>> that is simply non-solveable, even if you don't cache anything, it >>> will >>> still be racy. >> >> >> >> Absolutely -- I think solving this stuff 100% reliably without races >> isn't possible. I think the question is can we make this >> significantly >> better. > > > > > All, > > First of all, thanks for the feedback. This is indeed not a 100% > robust > solution, just something to allow tools like Asan to produce more sane > backtraces on crash (currently when stack is corrupted they would > crash > in > the unwinder without printing at least partial backtrace). > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. >>> >>> >>> >>> I'd argue that situation with __stack_chk_fail is very special. It's >>> used >>> in production code where you'd want to halt as soon as corruption is >>> detected to prevent further damage so even printing message is an >>> additional >>> risk. Debugging tools (e.g. sanitizers) are different and would prefer >>> to >>> print as many survived frames as possible. >>> >> >> But stack unwinder in libgcc is primarily for production use, not for >> debugging. > > > I can see it used in different projects to print backtraces on error > (bind9, > SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder > and > seems to be meant primarily for debugging (at least many projects use it > for > this purpose: > https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same > for > libbacktrace which, as its README mentions, is meant to "print a detailed > backtrace when an error occurs or to gather detailed profiling > information". But it shouldn't be performed on a corrupted stack. When a stack is corrupted, there is just no way to reliably unwind it. >>> >>> Why not? Attached patch shows that it's possible (up to a point of >>> corruption of course). >>> It isn't a problem for a debugger which is designed to analyze corrupted program. >>> >>> Yes but forcing all applications that would like to print helpful >>> message on stack corruption to employ gdb sounds less efficient that >>> providing f
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 2:01 PM, H.J. Lu wrote: > On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov > wrote: >> On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: >>> On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov >>> wrote: On 02.08.2017 23:04, H.J. Lu wrote: > > On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov > wrote: >> >> On 02.08.2017 21:48, H.J. Lu wrote: >>> >>> >>> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >>> >>> wrote: On 02.08.2017 20:02, Jeff Law wrote: > > > > On 08/02/2017 12:47 PM, Jakub Jelinek wrote: >> >> >> >> On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >>> >>> >>> >>> On 07/17/2017 01:23 AM, Yuri Gribov wrote: I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >>> >>> >>> >>> Is his stuff used for exception handling? If so, doesn't that make >>> the >>> performance a significant concern (ie that msync call?) >> >> >> >> >> For the performance issue, I wonder if it wouldn't be better to just >> compile the unwinder twice, basically include everything needed for >> the >> verification backtrace in a single *.c file with special defines, and >> not export anything from that *.o file except the single entrypoint. >> That would also handle the ABI problems. > > > > Yea. > > Given that the vast majority of the time the addresses are valid, I > wonder if we could take advantage of that property to keep the > overhead > lower in the most common cases. > >> For the concurrent calls of other threads unmapping stacks of running >> threads (that is where most of the verification goes against), I'm >> afraid >> that is simply non-solveable, even if you don't cache anything, it >> will >> still be racy. > > > > Absolutely -- I think solving this stuff 100% reliably without races > isn't possible. I think the question is can we make this > significantly > better. All, First of all, thanks for the feedback. This is indeed not a 100% robust solution, just something to allow tools like Asan to produce more sane backtraces on crash (currently when stack is corrupted they would crash in the unwinder without printing at least partial backtrace). >>> >>> FWIW, glibc 2.26 is changed to abort as soon as possible when stack is >>> corrupted: >>> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >>> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >>> >>> There is very little point to unwind stack when stack is corrupted. >> >> >> >> I'd argue that situation with __stack_chk_fail is very special. It's >> used >> in production code where you'd want to halt as soon as corruption is >> detected to prevent further damage so even printing message is an >> additional >> risk. Debugging tools (e.g. sanitizers) are different and would prefer >> to >> print as many survived frames as possible. >> > > But stack unwinder in libgcc is primarily for production use, not for > debugging. I can see it used in different projects to print backtraces on error (bind9, SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and seems to be meant primarily for debugging (at least many projects use it for this purpose: https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for libbacktrace which, as its README mentions, is meant to "print a detailed backtrace when an error occurs or to gather detailed profiling information". >>> >>> But it shouldn't be performed on a corrupted stack. When a stack is >>> corrupted, there is just no way to reliably unwind it. >> >> Why not? Attached patch shows that it's possible (up to a point of >> corruption of course). >> >>> It isn't a problem >>> for a debugger which is designed to analyze corrupted program. >> >> Yes but forcing all applications that would like to print helpful >> message on stack corruption to employ gdb sounds less efficient that >> providing fail-safe APIs in standard library... > ^^^ There is no such a thing of fail-safe on > corrupted > stack. A bad, but valid address, can be put on corrupted stack. When > you unwind it, the unwinder may not cr
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 5:52 AM, Yury Gribov wrote: > On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: >> On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov >> wrote: >>> On 02.08.2017 23:04, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: > > On 02.08.2017 21:48, H.J. Lu wrote: >> >> >> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >> >> wrote: >>> >>> >>> On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > > > > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> >> >> >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> >>> >>> >>> I've rebased the previous patch to trunk per Andrew's suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> >> >> >> Is his stuff used for exception handling? If so, doesn't that make >> the >> performance a significant concern (ie that msync call?) > > > > > For the performance issue, I wonder if it wouldn't be better to just > compile the unwinder twice, basically include everything needed for > the > verification backtrace in a single *.c file with special defines, and > not export anything from that *.o file except the single entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. > For the concurrent calls of other threads unmapping stacks of running > threads (that is where most of the verification goes against), I'm > afraid > that is simply non-solveable, even if you don't cache anything, it > will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. >>> >>> >>> >>> >>> All, >>> >>> First of all, thanks for the feedback. This is indeed not a 100% >>> robust >>> solution, just something to allow tools like Asan to produce more sane >>> backtraces on crash (currently when stack is corrupted they would crash >>> in >>> the unwinder without printing at least partial backtrace). >>> >> >> FWIW, glibc 2.26 is changed to abort as soon as possible when stack is >> corrupted: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >> >> There is very little point to unwind stack when stack is corrupted. > > > > I'd argue that situation with __stack_chk_fail is very special. It's > used > in production code where you'd want to halt as soon as corruption is > detected to prevent further damage so even printing message is an > additional > risk. Debugging tools (e.g. sanitizers) are different and would prefer > to > print as many survived frames as possible. > But stack unwinder in libgcc is primarily for production use, not for debugging. >>> >>> >>> I can see it used in different projects to print backtraces on error (bind9, >>> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and >>> seems to be meant primarily for debugging (at least many projects use it for >>> this purpose: >>> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for >>> libbacktrace which, as its README mentions, is meant to "print a detailed >>> backtrace when an error occurs or to gather detailed profiling information". >> >> But it shouldn't be performed on a corrupted stack. When a stack is >> corrupted, there is just no way to reliably unwind it. > > Why not? Attached patch shows that it's possible (up to a point of > corruption of course). > >> It isn't a problem >> for a debugger which is designed to analyze corrupted program. > > Yes but forcing all applications that would like to print helpful > message on stack corruption to employ gdb sounds less efficient that > providing fail-safe APIs in standard library... ^^^ There is no such a thing of fail-safe on corrupted stack. A bad, but valid address, can be put on corrupted stack. When you unwind it, the unwinder may not crash, but give you bogus stack backtrace. -- H.J.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Thu, Aug 3, 2017 at 1:22 PM, H.J. Lu wrote: > On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov > wrote: >> On 02.08.2017 23:04, H.J. Lu wrote: >>> >>> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >>> wrote: On 02.08.2017 21:48, H.J. Lu wrote: > > > On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov > > wrote: >> >> >> On 02.08.2017 20:02, Jeff Law wrote: >>> >>> >>> >>> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: > > > > On 07/17/2017 01:23 AM, Yuri Gribov wrote: >> >> >> >> I've rebased the previous patch to trunk per Andrew's suggestion. >> Original patch description/motivation/questions are in >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html > > > > Is his stuff used for exception handling? If so, doesn't that make > the > performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. >>> >>> >>> >>> Yea. >>> >>> Given that the vast majority of the time the addresses are valid, I >>> wonder if we could take advantage of that property to keep the >>> overhead >>> lower in the most common cases. >>> For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. >>> >>> >>> >>> Absolutely -- I think solving this stuff 100% reliably without races >>> isn't possible. I think the question is can we make this >>> significantly >>> better. >> >> >> >> >> All, >> >> First of all, thanks for the feedback. This is indeed not a 100% >> robust >> solution, just something to allow tools like Asan to produce more sane >> backtraces on crash (currently when stack is corrupted they would crash >> in >> the unwinder without printing at least partial backtrace). >> > > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is > corrupted: > > https://sourceware.org/bugzilla/show_bug.cgi?id=21752 > https://sourceware.org/bugzilla/show_bug.cgi?id=12189 > > There is very little point to unwind stack when stack is corrupted. I'd argue that situation with __stack_chk_fail is very special. It's used in production code where you'd want to halt as soon as corruption is detected to prevent further damage so even printing message is an additional risk. Debugging tools (e.g. sanitizers) are different and would prefer to print as many survived frames as possible. >>> >>> But stack unwinder in libgcc is primarily for production use, not for >>> debugging. >> >> >> I can see it used in different projects to print backtraces on error (bind9, >> SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and >> seems to be meant primarily for debugging (at least many projects use it for >> this purpose: >> https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for >> libbacktrace which, as its README mentions, is meant to "print a detailed >> backtrace when an error occurs or to gather detailed profiling information". > > But it shouldn't be performed on a corrupted stack. When a stack is > corrupted, there is just no way to reliably unwind it. Why not? Attached patch shows that it's possible (up to a point of corruption of course). > It isn't a problem > for a debugger which is designed to analyze corrupted program. Yes but forcing all applications that would like to print helpful message on stack corruption to employ gdb sounds less efficient that providing fail-safe APIs in standard library... >> Validation is also performed by libunwind (at least in some cases) which >> libgcc unwinder mimics. >> >> >>> Use it to unwind corrupted stack isn't appropriate. Santizer can try >>> similar >>> similar to valgrind to invoke a debugger to unwind corrupted stack. >> >> >> -Y > > > > -- > H.J.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Wed, Aug 2, 2017 at 9:29 PM, Yury Gribov wrote: > On 02.08.2017 23:04, H.J. Lu wrote: >> >> On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov >> wrote: >>> >>> On 02.08.2017 21:48, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: > > > On 02.08.2017 20:02, Jeff Law wrote: >> >> >> >> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: >>> >>> >>> >>> On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: > > > > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) >>> >>> >>> >>> >>> For the performance issue, I wonder if it wouldn't be better to just >>> compile the unwinder twice, basically include everything needed for >>> the >>> verification backtrace in a single *.c file with special defines, and >>> not export anything from that *.o file except the single entrypoint. >>> That would also handle the ABI problems. >> >> >> >> Yea. >> >> Given that the vast majority of the time the addresses are valid, I >> wonder if we could take advantage of that property to keep the >> overhead >> lower in the most common cases. >> >>> For the concurrent calls of other threads unmapping stacks of running >>> threads (that is where most of the verification goes against), I'm >>> afraid >>> that is simply non-solveable, even if you don't cache anything, it >>> will >>> still be racy. >> >> >> >> Absolutely -- I think solving this stuff 100% reliably without races >> isn't possible. I think the question is can we make this >> significantly >> better. > > > > > All, > > First of all, thanks for the feedback. This is indeed not a 100% > robust > solution, just something to allow tools like Asan to produce more sane > backtraces on crash (currently when stack is corrupted they would crash > in > the unwinder without printing at least partial backtrace). > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. >>> >>> >>> >>> I'd argue that situation with __stack_chk_fail is very special. It's >>> used >>> in production code where you'd want to halt as soon as corruption is >>> detected to prevent further damage so even printing message is an >>> additional >>> risk. Debugging tools (e.g. sanitizers) are different and would prefer >>> to >>> print as many survived frames as possible. >>> >> >> But stack unwinder in libgcc is primarily for production use, not for >> debugging. > > > I can see it used in different projects to print backtraces on error (bind9, > SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and > seems to be meant primarily for debugging (at least many projects use it for > this purpose: > https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for > libbacktrace which, as its README mentions, is meant to "print a detailed > backtrace when an error occurs or to gather detailed profiling information". But it shouldn't be performed on a corrupted stack. When a stack is corrupted, there is just no way to reliably unwind it. It isn't a problem for a debugger which is designed to analyze corrupted program. > Validation is also performed by libunwind (at least in some cases) which > libgcc unwinder mimics. > > >> Use it to unwind corrupted stack isn't appropriate. Santizer can try >> similar >> similar to valgrind to invoke a debugger to unwind corrupted stack. > > > -Y -- H.J.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 02.08.2017 23:04, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: On 02.08.2017 21:48, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. All, First of all, thanks for the feedback. This is indeed not a 100% robust solution, just something to allow tools like Asan to produce more sane backtraces on crash (currently when stack is corrupted they would crash in the unwinder without printing at least partial backtrace). FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. I'd argue that situation with __stack_chk_fail is very special. It's used in production code where you'd want to halt as soon as corruption is detected to prevent further damage so even printing message is an additional risk. Debugging tools (e.g. sanitizers) are different and would prefer to print as many survived frames as possible. But stack unwinder in libgcc is primarily for production use, not for debugging. I can see it used in different projects to print backtraces on error (bind9, SimGrid, sanitizers). backtrace(3) also sits on top of libgcc unwinder and seems to be meant primarily for debugging (at least many projects use it for this purpose: https://codesearch.debian.net/search?q=backtrace_symbols&perpkg=1). Same for libbacktrace which, as its README mentions, is meant to "print a detailed backtrace when an error occurs or to gather detailed profiling information". Validation is also performed by libunwind (at least in some cases) which libgcc unwinder mimics. Use it to unwind corrupted stack isn't appropriate. Santizer can try similar similar to valgrind to invoke a debugger to unwind corrupted stack. -Y
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Wed, Aug 2, 2017 at 1:56 PM, Yury Gribov wrote: > On 02.08.2017 21:48, H.J. Lu wrote: >> >> On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov >> wrote: >>> >>> On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > > > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> >> >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> >>> >>> I've rebased the previous patch to trunk per Andrew's suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> >> >> Is his stuff used for exception handling? If so, doesn't that make >> the >> performance a significant concern (ie that msync call?) > > > > For the performance issue, I wonder if it wouldn't be better to just > compile the unwinder twice, basically include everything needed for the > verification backtrace in a single *.c file with special defines, and > not export anything from that *.o file except the single entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. > For the concurrent calls of other threads unmapping stacks of running > threads (that is where most of the verification goes against), I'm > afraid > that is simply non-solveable, even if you don't cache anything, it will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. >>> >>> >>> >>> All, >>> >>> First of all, thanks for the feedback. This is indeed not a 100% robust >>> solution, just something to allow tools like Asan to produce more sane >>> backtraces on crash (currently when stack is corrupted they would crash >>> in >>> the unwinder without printing at least partial backtrace). >>> >> >> FWIW, glibc 2.26 is changed to abort as soon as possible when stack is >> corrupted: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21752 >> https://sourceware.org/bugzilla/show_bug.cgi?id=12189 >> >> There is very little point to unwind stack when stack is corrupted. > > > I'd argue that situation with __stack_chk_fail is very special. It's used > in production code where you'd want to halt as soon as corruption is > detected to prevent further damage so even printing message is an additional > risk. Debugging tools (e.g. sanitizers) are different and would prefer to > print as many survived frames as possible. > But stack unwinder in libgcc is primarily for production use, not for debugging. Use it to unwind corrupted stack isn't appropriate. Santizer can try similar similar to valgrind to invoke a debugger to unwind corrupted stack. -- H.J.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
* Jeff Law: > Something like setup a signal handler when we first start unwinding that > flags the error and tear it down when we're done unwinding?Obviously > we can't do setup/tear down each time for each address. Anyway, just > thinking outloud here... Linux doesn't have per-thread signal handlers, so this doesn't work reliably. If speed is not a concern, but reliability is, call fork (the system call, not glibc's wrapper which calls fork handlers) and do the work in a single-threaded copy of the process. There, you can set up signal handlers as you see fit, and the VM layout won't change unexpectedly. A completely different way to deal with this is to have the shell and abrt/apport/systemd-coredumpd coordinate and generate the backtrace from a userspace coredump handler. To harden unwinding against corrupted tables or table locations, we'd have to change ld.so to make all critical data read-only after loading and remove the unwinder caches (with more help from ld.so instead). It would make sense to move the unwinder implementation into ld.so. With proper hardening, corrupted stacks would not be able to cause crashes anymore, either.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 02.08.2017 21:48, H.J. Lu wrote: On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. All, First of all, thanks for the feedback. This is indeed not a 100% robust solution, just something to allow tools like Asan to produce more sane backtraces on crash (currently when stack is corrupted they would crash in the unwinder without printing at least partial backtrace). FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. I'd argue that situation with __stack_chk_fail is very special. It's used in production code where you'd want to halt as soon as corruption is detected to prevent further damage so even printing message is an additional risk. Debugging tools (e.g. sanitizers) are different and would prefer to print as many survived frames as possible. -Y
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Wed, Aug 2, 2017 at 1:39 PM, Yury Gribov wrote: > On 02.08.2017 20:02, Jeff Law wrote: >> >> On 08/02/2017 12:47 PM, Jakub Jelinek wrote: >>> >>> On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: > > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) >>> >>> >>> For the performance issue, I wonder if it wouldn't be better to just >>> compile the unwinder twice, basically include everything needed for the >>> verification backtrace in a single *.c file with special defines, and >>> not export anything from that *.o file except the single entrypoint. >>> That would also handle the ABI problems. >> >> Yea. >> >> Given that the vast majority of the time the addresses are valid, I >> wonder if we could take advantage of that property to keep the overhead >> lower in the most common cases. >> >>> For the concurrent calls of other threads unmapping stacks of running >>> threads (that is where most of the verification goes against), I'm afraid >>> that is simply non-solveable, even if you don't cache anything, it will >>> still be racy. >> >> Absolutely -- I think solving this stuff 100% reliably without races >> isn't possible. I think the question is can we make this significantly >> better. > > > All, > > First of all, thanks for the feedback. This is indeed not a 100% robust > solution, just something to allow tools like Asan to produce more sane > backtraces on crash (currently when stack is corrupted they would crash in > the unwinder without printing at least partial backtrace). > FWIW, glibc 2.26 is changed to abort as soon as possible when stack is corrupted: https://sourceware.org/bugzilla/show_bug.cgi?id=21752 https://sourceware.org/bugzilla/show_bug.cgi?id=12189 There is very little point to unwind stack when stack is corrupted. -- H.J.
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 02.08.2017 20:02, Jeff Law wrote: On 08/02/2017 12:47 PM, Jakub Jelinek wrote: On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: On 07/17/2017 01:23 AM, Yuri Gribov wrote: I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. All, First of all, thanks for the feedback. This is indeed not a 100% robust solution, just something to allow tools like Asan to produce more sane backtraces on crash (currently when stack is corrupted they would crash in the unwinder without printing at least partial backtrace). I probly put this aside for now given that people in community don't find it too useful. Should I close PR 67336 then? > Something like setup a signal handler when we first start unwinding that > flags the error and tear it down when we're done unwinding?Obviously > we can't do setup/tear down each time for each address. Anyway, just > thinking outloud here... Hm, I think I could sigsetjmp for SIGSEGV/SIGBUS before calling unwinder and then * siglongjmp back on error (if signal is caught in unwinding thread) * call users handler (for all other threads) It's not clear how this is going to work in presense of nested signals though. I could clear sigprocmask during execution of safe unwinding to prevent them but this sounds too gross... -Y
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 08/02/2017 12:47 PM, Jakub Jelinek wrote: > On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: >> On 07/17/2017 01:23 AM, Yuri Gribov wrote: >>> I've rebased the previous patch to trunk per Andrew's suggestion. >>> Original patch description/motivation/questions are in >>> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html >> Is his stuff used for exception handling? If so, doesn't that make the >> performance a significant concern (ie that msync call?) > > For the performance issue, I wonder if it wouldn't be better to just > compile the unwinder twice, basically include everything needed for the > verification backtrace in a single *.c file with special defines, and > not export anything from that *.o file except the single entrypoint. > That would also handle the ABI problems. Yea. Given that the vast majority of the time the addresses are valid, I wonder if we could take advantage of that property to keep the overhead lower in the most common cases. Something like setup a signal handler when we first start unwinding that flags the error and tear it down when we're done unwinding?Obviously we can't do setup/tear down each time for each address. Anyway, just thinking outloud here... > > For the concurrent calls of other threads unmapping stacks of running > threads (that is where most of the verification goes against), I'm afraid > that is simply non-solveable, even if you don't cache anything, it will > still be racy. Absolutely -- I think solving this stuff 100% reliably without races isn't possible. I think the question is can we make this significantly better. Jeff
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On Wed, Aug 02, 2017 at 12:38:13PM -0600, Jeff Law wrote: > On 07/17/2017 01:23 AM, Yuri Gribov wrote: > > I've rebased the previous patch to trunk per Andrew's suggestion. > > Original patch description/motivation/questions are in > > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html > Is his stuff used for exception handling? If so, doesn't that make the > performance a significant concern (ie that msync call?) For the performance issue, I wonder if it wouldn't be better to just compile the unwinder twice, basically include everything needed for the verification backtrace in a single *.c file with special defines, and not export anything from that *.o file except the single entrypoint. That would also handle the ABI problems. For the concurrent calls of other threads unmapping stacks of running threads (that is where most of the verification goes against), I'm afraid that is simply non-solveable, even if you don't cache anything, it will still be racy. Jakub
Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind
On 07/17/2017 01:23 AM, Yuri Gribov wrote: > Hi all, > > I've rebased the previous patch to trunk per Andrew's suggestion. > Original patch description/motivation/questions are in > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html Is his stuff used for exception handling? If so, doesn't that make the performance a significant concern (ie that msync call?) Though I wouldn't be a fan of parsing /proc/self/maps either. Given something like msync or parsing /proc/self/maps, aren't you essentially limiting this to Linux targets? Which means you probably ought to be using getpagesize() to get the page size rather than pulling it from a config file or hardcoding it. You're #including sys/mman.h, shouldn't that be protected with a HAVE_SYS_MMAN_H and the autoconf hackery that implies? Is the extension of struct _Unwind_Context an ABI change? ie, can this co-exist with existing code? Note that I'm not entirely sure caching is appropriate/safe. Consider if we've got multiple threads. One is in the middle of unwinding while another happens to be unmapping pages. I believe the Red Hat bz has a bug of this nature that's still not fully analyzed. Not a full review -- this stuff is well outside my areas of expertise. BUt I do happen to have some state via the Red Hat BZ that I looked at a while ago :-) jeff
[RFC][PR 67336][PING^6] Verify pointers during stack unwind
Hi all, I've rebased the previous patch to trunk per Andrew's suggestion. Original patch description/motivation/questions are in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01869.html -Y safe-unwind-2.patch Description: Binary data #include #include struct _Unwind_Context; typedef int (*_Unwind_Trace_Fn)(struct _Unwind_Context *, void *vdata); extern int _Unwind_Backtrace(_Unwind_Trace_Fn trace, void * trace_argument); extern int _Unwind_Backtrace_Checked(_Unwind_Trace_Fn trace, void * trace_argument); #ifdef CHECK_UNWIND #define _Unwind_Backtrace _Unwind_Backtrace_Checked #endif extern void *_Unwind_GetIP (struct _Unwind_Context *context); int simple_unwind (struct _Unwind_Context *context, void *vdata) { printf("Next frame: "); void *pc = _Unwind_GetIP(context); printf("%p\n", pc); return 0; } #define noinline __attribute__((noinline)) noinline int foo() { // Clobber stack to provoke errors in unwinder int x; void *p = &x; asm("" :: "r"(p)); memset(p, 0xa, 128); printf("After clobbering stack\n"); int ret = _Unwind_Backtrace(simple_unwind, 0); printf("After unwind: %d\n", ret); printf("We're going to fail now\n"); return 0; } noinline int bar() { int x = foo(); return x + 1; } int main() { bar(); return 0; }