Re: [PATCH] Avoid deadlock in malloc on backtrace

[Florian Weimer <fweimer at redhat dot com>]

On 02/24/2015 11:02 AM, Siddhesh Poyarekar wrote:
> When the malloc subsystem detects some kind of memory corruption, 
> depending on the configuration it prints the error, a backtrace, a 
> memory map and then aborts the process.  In this process, the 
> backtrace() call may result in a call to malloc, resulting in 
> various kinds of problematic behavior.

Hasn't GCC a backtrace library which does not use malloc?

> In one case, the malloc it calls may detect a corruption and call 
> backtrace again, and a stack overflow may result due to the
> infinite recursion.  In another case, the malloc it calls may
> deadlock on an arena lock with the malloc (or free, realloc, etc.)
> that detected the corruption.  In yet another case, if the program
> is linked with pthreads, backtrace may do a pthread_once
> initialization, which deadlocks on itself.
> 
> In all these cases, the program exit is not as intended.  This is 
> avoidable by marking the arena that malloc detected a corruption
> on, as unusable.

>From a security point of view, this is wrong.  glibc should not try to
attempt to do even more work if a heap corruption is detected.  The
proper fix would be to use a coredump handler to print the backtrace,
completely outside the process.  This also has the advantage that the
backtrace can use separate/compressed debugging information.

Maybe from a functionality point of view, this is the right thing to do.

The test case is invalid for multiple reasons: the compiler can detect
that the pointer arithmetic before the allocated buffer is invalid.
There is a use-after-free.  Maybe it's possible to fix this with
-ffreestanding; I don't know if the glibc headers obey that, though.

In the test case, the magic constant “8” should probably be replaced
with 2 * sizeof (size_t).  Please also add a comment what this test
case is testing, it is not immediately obvious.

-- 
Florian Weimer / Red Hat Product Security