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
>