Re: [PATCH] Avoid deadlock in malloc on backtrace

[Mike Frysinger <vapier at gentoo dot org>]

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.

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

> I'm against removing the corruption detection messages because they
> are useful for *me*, and when abrt doesn't work or doesn't work
> correctly (see 3).

agreed

> The best argument is that the default should be to call abort()
> immediately, and I don't disagree with that, but I still find the
> backtrace useful, and supportable, and would argue to keep it in
> place as a choice to developers and system integrators.

i also find this useful, but we probably want to make sure we don't spend too 
much time on a complicated solution.  the proposed patch didn't seem too bad to 
me.
-mike

Attachment: signature.asc
Description: Digital signature