Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240110015837.GQ4163@brightrain.aerifal.cx>
Date: Tue, 9 Jan 2024 20:58:37 -0500
From: Rich Felker <dalias@...c.org>
To: enh <enh@...gle.com>
Cc: musl@...ts.openwall.com, jvoisin <julien.voisin@...tri.org>
Subject: Re: Protect pthreads' mutexes against use-after-destroy

On Tue, Jan 09, 2024 at 03:27:37PM -0800, enh wrote:
> On Tue, Jan 9, 2024 at 11:07 AM Rich Felker <dalias@...c.org> wrote:
> >
> > On Tue, Jan 09, 2024 at 03:37:17PM +0100, jvoisin wrote:
> > > Ohai,
> > >
> > > as discussed on irc, Android's bionic has a check to prevent
> > > use-after-destroy on phtread mutexes
> > > (https://github.com/LineageOS/android_bionic/blob/e0aac7df6f58138dae903b5d456c947a3f8092ea/libc/bionic/pthread_mutex.cpp#L803),
> > > and musl doesn't.
> > >
> > > While odds are that this is a super-duper common bug, it would still be
> > > nice to have this kind of protection, since it's cheap, and would
> > > prevent/make it easy to diagnose weird states.
> > >
> > > Is this something that should/could be implemented?
> > >
> > > o/
> >
> > I think you meant that the odds are it's not common.
> 
> it was common enough (and hard enough to debug) that we added this
> "best effort" error detection to bionic :-)

Thanks! That's useful information (and disturbing...)

> > There's already
> > enough complexity in the code paths for supporting all the different
> > mutex types that my leaning would be, if we do any hardening for
> > use-after-destroy, that it should probably just take the form of
> > putting the object in a state that will naturally deadlock or error
> > rather than adding extra checks to every path where it's used.
> 
> yeah, the _other_ reason we have the abort is that we've struggled
> over the years to make it clear to the _callers_ that -- just because
> you crash/hang in libc -- it's the _caller's_ bug. explicitly saying
> so helps. (though we still get a decent number of people who don't
> read/don't understand.)

That's a problem we've just not tried to address in musl, but indeed
it does happen fairly often. If we did try at some point, I think it
would be by setting up a state to make the reason more clear in the
debugger, not producing any output. But personally I'd like to keep
driving home the message that the point of crash does not mean
anything about responsibility for the crash, even if it means
responding to lots of bogus "bug reports"...

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.