[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZauAbDW5gvvb6iDM@voyager>
Date: Sat, 20 Jan 2024 09:12:28 +0100
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: [PATCH] Split fork and abort locks
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.
Anyway, with posix_spawn() taking __abort_lock, when it came time to
guard against leaking communication pipes to concurrently created child
processes, it was only natural to use the same lock in clone() and
_Fork(). But this is a new purpose and deserves a new lock.
So I am submitting a patch to introduce that lock.
Ciao,
Markus
View attachment "0001-Introduce-new-fork-lock-separate-from-__abort_lock.patch" of type "text/x-diff" (3881 bytes)
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.
