[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4da16095016feeccc01d23b3b29533f5@ispras.ru>
Date: Sat, 20 Jan 2024 14:29:28 +0300
From: Alexey Izbyshev <izbyshev@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Split fork and abort locks
On 2024-01-20 11:12, Markus Wichmann wrote:
> Hi all,
>
> a while ago I had noticed that __abort_lock was being taken in some
> functions that have nothing to do with SIGABRT. Namely in the forking
> functions. Investigating this a bit, I noticed that __abort_lock had
> become dual purpose. But this is a code smell.
>
> Actually, there are several locks that have expanded in scope a bit
> since their introduction. At least the ptc lock (__inhibit_ptc() et
> al.)
> deserves a closer look later on as well. Seems to me like in case of
> the
> default stack size, that lock is used simply because an rwlock was
> needed and this one was around.
>
> The corruption in this case probably came from posix_spawn(). That
> function takes the abort lock, because, as a baffling comment above the
> lock statement tells us, this guards against SIGABRT disposition
> changing. This is because abort() changes the disposition without
> calling sigaction(), so a handler would not be noted in the handler
> set.
>
> However, actually posix_spawn() does not need to care about this. The
> handler set is strictly additive, so all the signals it contains /may/
> have a handler. And abort() removes a handler. In the worst case, the
> handler set will spuriously contain SIGABRT, which is a condition the
> child needs to check for anyway.
>
> And a concurrent sigaction() call from the application establishing a
> handler is no different for SIGABRT than for any other signal. That is
> handled by retrieving the handler set in the child, when everything is
> fixed since the child is single-threaded. For the same reason, there
> cannot be a concurrent call to abort() in the child.
>
The problem that __abort_lock solves is that a child process created by
fork/_Fork/clone/posix_spawn should not observe SIGABRT disposition
reset if abort is called by the parent concurrently with the child
creation. For example, if the initial SIGABRT disposition is SIG_IGN,
and one thread of a program calls posix_spawn while another thread calls
abort, without __abort_lock the child could inherit SIG_DFL disposition.
This is not related to maintaining handler_set used for posix_spawn
optimization.
A separate reason for why removing __abort_lock LOCK/UNLOCK from clone.c
and _Fork.c (as done in your patch) is wrong is because they take part
in creation of a consistent execution state in the child (the child
should be able to call abort and not deadlock).
Alexey
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.
