Re: [PATCH] Avoid deadlock in malloc on backtrace

[Mike Frysinger <vapier at gentoo dot org>]

On 02 Mar 2015 12:34, Rich Felker wrote:
> On Mon, Mar 02, 2015 at 12:30:51AM -0500, Mike Frysinger wrote:
> > On 26 Feb 2015 17:18, Carlos O'Donell wrote:
> > > On 02/24/2015 05: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.
> > > 
> > > There have been various comments about this patch and I would like to
> > > list some supporting rationale for this patch:
> > > 
> > > (1) Delaying the abort is bad for security.
> > > 
> > > Problem: The library should abort() immediately from a security perspective.
> > > 
> > > Solution: Don't delay, call abort() immediately.
> > > 
> > > We all agree that delaying the abort is bad from a security perspective.
> > > However, the present solution is a trade-off between providing useful
> > > diagnostics and *then* aborting.
> > 
> > i think the current line glibc attempts to walk is reasonable.  there are other
> > funcs which are arguably more important (__chk_fail & __fortify_fail) that do a
> > good amount of work (calling into __libc_message).  from a security pov, that is 
> > all bad mojo.
> 
> And they should all be fixed.

to be clear, while i support adding an optional "paranoid" mode, i'm entirely 
against disabling them by default.  these crash messages have improved the lives 
of users and developers significantly since they were added.  i don't have data 
to back this claim up, but having worked as a developer writing my own code, 
an upstream developer releasing my own packages, and a distro maintainer on a 
wide swath of packages, all continuously since the glibc 2.2.5 days, i think my 
anecdotal memory is sufficient.  as a maintainer of the toolchain, i certainly 
know it helps bucket bugs -- before this message, most random crashes were 
thrown at the toolchain distro maintainers (after all, a crash in malloc must 
surely be the fault of the C library).  now with all the memory corruption 
hooks, we've got a pretty strong/reliable signal that it is entirely the fault 
of the package.  returning to that status quo would be a terrible idea.

if a distro has frameworks in place to replace the functionality of these (e.g. 
a core dump handler), then they can make the decision to also opt in their C 
library to this mode.  on Gentoo, we make the decision based on how hardened the 
user wants their system.

> > it might be nice if there was a configure option to enable a paranoid mode 
> > whereby we die as safely as possible when a failure is detected.  in Gentoo we 
> > have a custom handler that uses direct syscalls and logs to the system logger
> > before killing itself (SIGABRT & SIGKILL).
> 
> Even syscalls are unsafe on x86 unless you use custom asm. The default
> mechanism goes through a syscall vdso pointer in the TCB at %gs:...

i'm aware ... we do force the int80 method in some places because of interaction 
with gs setup as the code will crash otherwise during early init.  although this 
particular case doesn't seem to be accounting for that, so i'll update it too.
-mike

Attachment: signature.asc
Description: Digital signature