Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[Yury Gribov]

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

[Jeff Law]

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

[Florian Weimer]

* 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

[Florian Weimer]

* 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

[Jakub Jelinek]

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

[Yury Gribov]

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=1). 
>>> 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[Jeff Law]

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

[Jeff Law]

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=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

[H.J. Lu]

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=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
> 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[Jeff Law]

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=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 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[H.J. Lu]

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:
>>> 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[Yury Gribov]

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=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 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[H.J. Lu]

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=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 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[Yury Gribov]

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=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 

Re: [RFC][PR 67336][PING^6] Verify pointers during stack unwind

[H.J. Lu]

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=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

[Yury Gribov]

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=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

[H.J. Lu]

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=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

[Yury Gribov]

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=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

[H.J. Lu]

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

[Florian Weimer]

* 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

[Yury Gribov]

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

[H.J. Lu]

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

[Yury Gribov]

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

[Jeff Law]

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

[Jakub Jelinek]

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

[Jeff Law]

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