Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201001143918.GN17637@brightrain.aerifal.cx>
Date: Thu, 1 Oct 2020 10:39:19 -0400
From: Rich Felker <dalias@...c.org>
To: Florian Weimer <fw@...eb.enyo.de>
Cc: Carlos O'Donell via Libc-alpha <libc-alpha@...rceware.org>,
	musl@...ts.openwall.com
Subject: Re: Re: [PATCH] Make abort() AS-safe (Bug 26275).

On Thu, Oct 01, 2020 at 08:08:24AM +0200, Florian Weimer wrote:
> * Rich Felker:
> 
> > Even without fork, execve and posix_spawn can also see the SIGABRT
> > disposition change made by abort(), passing it on to a process that
> > should have started with a disposition of SIG_IGN if you hit exactly
> > the wrong spot in the race.
> 
> My feeling is that it's not worth bothering with this kind of leakage.
> We've had this bug forever in glibc, and no one has complained about
> it.
> 
> Carlos is investigating removal of the abort lock from glibc, I think.

I don't think that's a good solution. The lock is really important in
that it protects against serious wrong behavior *within the process*
like an application-installed signal handler for SIGABRT getting
called more than once. The worst outcome of the exec issue discussed
here (and similar for fork) is simply the wrong disposition (SIG_DFL
rather than SIG_IGN) getting passed on to the new process image -- and
it's very rare to actually want SIG_IGN to be inherited. Undoing an
important fix for wrong code execution just because it's an incomplete
fix for wrong disposition inheritance would be bad.

If we want to solve this without relying on broadcast/seizure of all
threads, it looks like it may be a tradeoff between the subtly wrong
behavior for inheritance of SIG_IGN, and subtle wrongness with respect
to POSIX requirements on termination status from abort. It's possible
to just probe whether the old status was SIG_IGN; if not, there's no
issue with the current approach of changing it to SIG_DFL. But if it
was SIG_IGN, you can sacrifice the desired SIGABRT termination and get
termination by SIGSEGV or SIGILL or SIGBUS (still abnormal status,
still generating a core, just not the right signal number) just by
leaving signals masked and executing an instruction that will fault in
one of these ways.

FWIW musl "already does this" (attempting to terminate via a_crash()
with signals still masked) if the SIGABRT after resetting its
disposition somehow fails to terminate the process. It then falls back
to raising SIGKILL, SYS_exit_group, SYS_exit, and for(;;), so we'd get
this behavior automatically just by skipping the sigaction if the old
disposition was SIG_IGN.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.