How to use backtrace() safely in signal handler.

How to use backtrace() safely in signal handler.

[Paul Guo]

Hello,

I know it is Async signal unsafe per
https://www.gnu.org/software/libc/manual/html_node/Backtraces.html

Function: int backtrace (void **buffer, int size)

Preliminary: | MT-Safe | AS-Unsafe init heap dlopen plugin lock | AC-Unsafe
init mem lock fd | See POSIX Safety

but I want to know how unsafe it is and see if I could use that safely in
signal handler with some limitations.

I found the explanation of those words like "heap", "dlopen", etc from
https://www.gnu.org/software/libc/manual/html_node/Unsafe-Features.html

But is still a bit confusing. So I have some questions as below,

1. what is "init" in the AS-Unsafe words? It is not explained in the above
URL.

2. heap

The doc says "Functions marked with heap may call heap memory management
functions from the malloc/free family of functions and are only as safe as
those functions"

But https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html
<https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html>says
backtrace() trigger malloc() only when dynamically loading libgcc. Does
that mean if libgcc was preloaded in the application we do not need to
worry about that?

And it looks like for glibc with that patch we do not even need to worry
about the 'heap' case. Am I correctly understanding?

2. dlopen: preloading libgcc could work around this limitation, right?

3. plugin: I do not know how backtrace() is affected by this but it seems
that it does not affect backtrace()?

4. lock: I've seen a self deadlock (in backtrace()) case in my environment
(backtrace()-ing in one signal handler and then gets another signal and
that signal handler calls backtrace() again). The lock should
be __gthread_mutex_lock. For this we could use a wrapper of backtrace() to
prevent reentrance of backtrace() to work around this issue (Yes of course
the final backtrace() will return nothing but this scenario is rare so it
should be fine, right?).

Thanks,
Paul

Re: How to use backtrace() safely in signal handler.

[Carlos O'Donell]

On Thu, Dec 5, 2019 at 8:16 AM Paul Guo <paulguo@gmail.com> wrote:
> but I want to know how unsafe it is and see if I could use that safely in
> signal handler with some limitations.

Why do you want to call backtrace() from a signal handler?

In the past this was the accepted practice for doing crash backtrace handling.

In-process crash backtrace handling is a security risk and unsafe.

Is there any reason why you don't pursue out-of-process handling?

Cheers,
Carlos.

Re: How to use backtrace() safely in signal handler.

[Florian Weimer]

* Paul Guo:

> 1. what is "init" in the AS-Unsafe words? It is not explained in the above
> URL.

I think it refers to the first call which performs initialization.

> 2. heap
>
> The doc says "Functions marked with heap may call heap memory management
> functions from the malloc/free family of functions and are only as safe as
> those functions"

If objects have GNU-style unwinding data and nothing calls
__register_frame_info, the libgcc unwinder will not use malloc AFAICS.
But I'm not entirely sure what this annotation is about.

> But https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html
> <https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html>says
> backtrace() trigger malloc() only when dynamically loading libgcc. Does
> that mean if libgcc was preloaded in the application we do not need to
> worry about that?

This no longer applies to current glibc for a whole bunch of reasons.

> 2. dlopen: preloading libgcc could work around this limitation, right?

No, libgcc_s calls _dl_iterate_phdr, which needs the loader lock.  I'm
guessing the annotation refers to that.

> 3. plugin: I do not know how backtrace() is affected by this but it seems
> that it does not affect backtrace()?

It refers to libgcc_s.  libgcc_s has its own locking to support
__register_frame_info, but it is skipped if nothing calls this function.
It does however rely on the loader lock via _dl_iterate_phdr.

> 4. lock: I've seen a self deadlock (in backtrace()) case in my environment
> (backtrace()-ing in one signal handler and then gets another signal and
> that signal handler calls backtrace() again). The lock should
> be __gthread_mutex_lock. For this we could use a wrapper of backtrace() to
> prevent reentrance of backtrace() to work around this issue (Yes of course
> the final backtrace() will return nothing but this scenario is rare so it
> should be fine, right?).

With __register_frame_info, you can get other deadlocks on the internal
libgcc unwinder lock.

Nowadays, the loader write lock is recursive, so if the signal handler
interrupts certain dynamic loader operations, there won't be a deadlock
due to _dl_iterate_phdr, but you could get crashes because the signal
handler observes a temporarily corrupted list.

To fix this, we would have to change the dynamic linker to provide an
async-signal-safe function which returns the loaded object for an
address and its unwinding data.  Then we'd have to patch libgcc_s to use
that, in preference of its own caches.  This would be useful for C++
code which wants to catch exceptions in an asynchronous signal handler,
without letting them escape from the signal handler.

If this is just for a crash reporter, you really need to move this
out-of-process because in-process crash handlers will always be prone to
deadlocks and can obscure the actual source of the crash.

Thanks,
Florian

Re: How to use backtrace() safely in signal handler.

[Paul Guo]

On Thu, Dec 5, 2019 at 10:02 PM Carlos O'Donell <carlos@systemhalted.org>
wrote:

> On Thu, Dec 5, 2019 at 8:16 AM Paul Guo <paulguo@gmail.com> wrote:
> > but I want to know how unsafe it is and see if I could use that safely in
> > signal handler with some limitations.
>
> Why do you want to call backtrace() from a signal handler?
>
>

Typically program sigsegv handler wants a backtrace and saves them somwhere
and
then rethrows the signal (and then program quits). This is a very
convenient practice
and seems to be not unusual in some softwares.


In the past this was the accepted practice for doing crash backtrace
> handling.
>
> In-process crash backtrace handling is a security risk and unsafe.
>
> Is there any reason why you don't pursue out-of-process handling?
>
> Cheers,
> Carlos.
>

Re: How to use backtrace() safely in signal handler.

[Paul Guo]

On Thu, Dec 5, 2019 at 10:35 PM Florian Weimer <fweimer@redhat.com> wrote:

> * Paul Guo:
>
> > 1. what is "init" in the AS-Unsafe words? It is not explained in the
> above
> > URL.
>
> I think it refers to the first call which performs initialization.
>
> > 2. heap
> >
> > The doc says "Functions marked with heap may call heap memory management
> > functions from the malloc/free family of functions and are only as safe
> as
> > those functions"
>
> If objects have GNU-style unwinding data and nothing calls
> __register_frame_info, the libgcc unwinder will not use malloc AFAICS.
> But I'm not entirely sure what this annotation is about.
>
> > But https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html
> > <https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html>says
> > backtrace() trigger malloc() only when dynamically loading libgcc. Does
> > that mean if libgcc was preloaded in the application we do not need to
> > worry about that?
>
> This no longer applies to current glibc for a whole bunch of reasons.
>

OK. Do you mean with current glibc, if a program is calling malloc() and
gets a signal,
and if the signal handler calls backtrace(), backtrace() won't be safe?


> > 2. dlopen: preloading libgcc could work around this limitation, right?
>
> No, libgcc_s calls _dl_iterate_phdr, which needs the loader lock.  I'm
> guessing the annotation refers to that.
>
> > 3. plugin: I do not know how backtrace() is affected by this but it seems
> > that it does not affect backtrace()?
>
> It refers to libgcc_s.  libgcc_s has its own locking to support
> __register_frame_info, but it is skipped if nothing calls this function.
> It does however rely on the loader lock via _dl_iterate_phdr.
>
> > 4. lock: I've seen a self deadlock (in backtrace()) case in my
> environment
> > (backtrace()-ing in one signal handler and then gets another signal and
> > that signal handler calls backtrace() again). The lock should
> > be __gthread_mutex_lock. For this we could use a wrapper of backtrace()
> to
> > prevent reentrance of backtrace() to work around this issue (Yes of
> course
> > the final backtrace() will return nothing but this scenario is rare so it
> > should be fine, right?).
>
> With __register_frame_info, you can get other deadlocks on the internal
> libgcc unwinder lock.
>
> Nowadays, the loader write lock is recursive, so if the signal handler
> interrupts certain dynamic loader operations, there won't be a deadlock
> due to _dl_iterate_phdr, but you could get crashes because the signal
> handler observes a temporarily corrupted list.
>
> To fix this, we would have to change the dynamic linker to provide an
> async-signal-safe function which returns the loaded object for an
> address and its unwinding data.  Then we'd have to patch libgcc_s to use
> that, in preference of its own caches.  This would be useful for C++
> code which wants to catch exceptions in an asynchronous signal handler,
> without letting them escape from the signal handler.
>
>
OK. Can I summarize like this after reading replies for questions 2 & 3 & 4:

If the application is preloading some libraries, then if it gets a signal
and then
backtrace() in the signal handler won't not safe. Other signal unsafe issues
include 1) reentrance of backtrace() due to various possible locks or even
data
corruption. 2) the "heap" issue(If I understand correctly. See reply above),
In other cases backtrace() is ok as a signal handler?

If this is just for a crash reporter, you really need to move this
> out-of-process because in-process crash handlers will always be prone to
> deadlocks and can obscure the actual source of the crash.
>

What is the usual "out-of-process" practice?
  /proc/sys/kernel/core_pattern + abrt kind of configuration (take
rhel/centos distribution as an example)?

Thanks you and Carlos (previous replier) for response.   -- Paul

Re: How to use backtrace() safely in signal handler.

[Adhemerval Zanella]

On 05/12/2019 23:47, Paul Guo wrote:
> On Thu, Dec 5, 2019 at 10:35 PM Florian Weimer <fweimer@redhat.com> wrote:
> 
>> * Paul Guo:
>>
>>> 1. what is "init" in the AS-Unsafe words? It is not explained in the
>> above
>>> URL.
>>
>> I think it refers to the first call which performs initialization.
>>
>>> 2. heap
>>>
>>> The doc says "Functions marked with heap may call heap memory management
>>> functions from the malloc/free family of functions and are only as safe
>> as
>>> those functions"
>>
>> If objects have GNU-style unwinding data and nothing calls
>> __register_frame_info, the libgcc unwinder will not use malloc AFAICS.
>> But I'm not entirely sure what this annotation is about.
>>
>>> But https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html
>>> <https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html>says
>>> backtrace() trigger malloc() only when dynamically loading libgcc. Does
>>> that mean if libgcc was preloaded in the application we do not need to
>>> worry about that?
>>
>> This no longer applies to current glibc for a whole bunch of reasons.
>>
> 
> OK. Do you mean with current glibc, if a program is calling malloc() and
> gets a signal,
> and if the signal handler calls backtrace(), backtrace() won't be safe?

Yes, dlopen on generic backtrace.c will call malloc and glibc malloc
implementation is not async-signal-safe.

> 
> 
>>> 2. dlopen: preloading libgcc could work around this limitation, right?
>>
>> No, libgcc_s calls _dl_iterate_phdr, which needs the loader lock.  I'm
>> guessing the annotation refers to that.
>>
>>> 3. plugin: I do not know how backtrace() is affected by this but it seems
>>> that it does not affect backtrace()?
>>
>> It refers to libgcc_s.  libgcc_s has its own locking to support
>> __register_frame_info, but it is skipped if nothing calls this function.
>> It does however rely on the loader lock via _dl_iterate_phdr.
>>
>>> 4. lock: I've seen a self deadlock (in backtrace()) case in my
>> environment
>>> (backtrace()-ing in one signal handler and then gets another signal and
>>> that signal handler calls backtrace() again). The lock should
>>> be __gthread_mutex_lock. For this we could use a wrapper of backtrace()
>> to
>>> prevent reentrance of backtrace() to work around this issue (Yes of
>> course
>>> the final backtrace() will return nothing but this scenario is rare so it
>>> should be fine, right?).
>>
>> With __register_frame_info, you can get other deadlocks on the internal
>> libgcc unwinder lock.
>>
>> Nowadays, the loader write lock is recursive, so if the signal handler
>> interrupts certain dynamic loader operations, there won't be a deadlock
>> due to _dl_iterate_phdr, but you could get crashes because the signal
>> handler observes a temporarily corrupted list.
>>
>> To fix this, we would have to change the dynamic linker to provide an
>> async-signal-safe function which returns the loaded object for an
>> address and its unwinding data.  Then we'd have to patch libgcc_s to use
>> that, in preference of its own caches.  This would be useful for C++
>> code which wants to catch exceptions in an asynchronous signal handler,
>> without letting them escape from the signal handler.
>>
>>
> OK. Can I summarize like this after reading replies for questions 2 & 3 & 4:
> 
> If the application is preloading some libraries, then if it gets a signal
> and then
> backtrace() in the signal handler won't not safe. Other signal unsafe issues
> include 1) reentrance of backtrace() due to various possible locks or even
> data
> corruption. 2) the "heap" issue(If I understand correctly. See reply above),
> In other cases backtrace() is ok as a signal handler?

If you mean by 'preloading' the concept that if the thread issue
dlopen, dlclose, or dlmopen then yes, dl_iterate_phdr will see corrupted
data.  The reentracy of backtrace() is essentially the same reentracy
issue for malloc and dl_iterate_phdr. Another possible issue is a stack
overflow on signal handler due calling backtrace itself (due dlopen and/or
libgcc calls).

These are the known issues with backtrace on signal handler.

> 
> If this is just for a crash reporter, you really need to move this
>> out-of-process because in-process crash handlers will always be prone to
>> deadlocks and can obscure the actual source of the crash.
>>
> 
> What is the usual "out-of-process" practice?
>   /proc/sys/kernel/core_pattern + abrt kind of configuration (take
> rhel/centos distribution as an example)?

The abrt afaik just setup and handle the generated core file by
configuring the system required parameters. You will still need
a tool to read and process the core file, gdb for instance.

> 
> Thanks you and Carlos (previous replier) for response.   -- Paul
> 

Re: How to use backtrace() safely in signal handler.

[Florian Weimer]

* Paul Guo:

>  > But https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html
>  > <https://sourceware.org/ml/libc-alpha/2015-02/msg00659.html>says
>  > backtrace() trigger malloc() only when dynamically loading libgcc. Does
>  > that mean if libgcc was preloaded in the application we do not need to
>  > worry about that?
>
>  This no longer applies to current glibc for a whole bunch of reasons.
>
> OK. Do you mean with current glibc, if a program is calling malloc()
> and gets a signal, and if the signal handler calls backtrace(),
> backtrace() won't be safe?

The thread you referenced was not about that.

Whether the scenario you describe depends on a lot of circumstances.  It
can be made safe, but in general, it is not.

> OK. Can I summarize like this after reading replies for questions 2 & 3 & 4:
>
> If the application is preloading some libraries, then if it gets a
> signal and then backtrace() in the signal handler won't not
> safe. Other signal unsafe issues include 1) reentrance of backtrace()
> due to various possible locks or even data corruption. 2) the "heap"
> issue(If I understand correctly. See reply above), In other cases
> backtrace() is ok as a signal handler?

If you want to be compatible with future glibc versions, you cannot
assume that certain uses of backtrace are async-signal-safe.

>  If this is just for a crash reporter, you really need to move this
>  out-of-process because in-process crash handlers will always be prone to
>  deadlocks and can obscure the actual source of the crash.
>
>  
> What is the usual "out-of-process" practice?
>   /proc/sys/kernel/core_pattern + abrt kind of configuration (take rhel/centos distribution as an example)?

Yes, or analyzing plain old coredumps.

The kernel coredump hooks have the advantage that they can produce a
backtrace even without writing the core to disk.

Thanks,
Florian

Re: How to use backtrace() safely in signal handler.

[Florian Weimer]

* Adhemerval Zanella:

>> What is the usual "out-of-process" practice?
>>   /proc/sys/kernel/core_pattern + abrt kind of configuration (take
>> rhel/centos distribution as an example)?
>
> The abrt afaik just setup and handle the generated core file by
> configuring the system required parameters. You will still need
> a tool to read and process the core file, gdb for instance.

No, both ABRT and systemd contain tools to performan basic analysis,
such as capturing symbolic backtraces.  without every writing the
coredump to disk (which can be very large).

Thanks,
Florian

Re: How to use backtrace() safely in signal handler.

[Adhemerval Zanella]

On 09/12/2019 13:17, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> What is the usual "out-of-process" practice?
>>>   /proc/sys/kernel/core_pattern + abrt kind of configuration (take
>>> rhel/centos distribution as an example)?
>>
>> The abrt afaik just setup and handle the generated core file by
>> configuring the system required parameters. You will still need
>> a tool to read and process the core file, gdb for instance.
> 
> No, both ABRT and systemd contain tools to performan basic analysis,
> such as capturing symbolic backtraces.  without every writing the
> coredump to disk (which can be very large).

Interesting, I wasn't aware of that. How does it accomplish it? Linking
with libelf or something related and analysing the code itself?

Re: How to use backtrace() safely in signal handler.

[Florian Weimer]

* Adhemerval Zanella:

> On 09/12/2019 13:17, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>>> What is the usual "out-of-process" practice?
>>>>   /proc/sys/kernel/core_pattern + abrt kind of configuration (take
>>>> rhel/centos distribution as an example)?
>>>
>>> The abrt afaik just setup and handle the generated core file by
>>> configuring the system required parameters. You will still need
>>> a tool to read and process the core file, gdb for instance.
>> 
>> No, both ABRT and systemd contain tools to performan basic analysis,
>> such as capturing symbolic backtraces.  without every writing the
>> coredump to disk (which can be very large).
>
> Interesting, I wasn't aware of that. How does it accomplish it? Linking
> with libelf or something related and analysing the code itself?

Yes, I think the backtraces come from <https://github.com/abrt/satyr>.
The kernel enables this because you can mmap stdin with the coredump
in the coredump handler (or you can poke at /proc).

Thanks,
Florian

Re: How to use backtrace() safely in signal handler.

[Paul Guo]

OK. Based on the discussion, I think we can conclude that using backtrace()
in signal handler is
not safe even with various "workaround". Thanks guys for help.