[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170601161638.GL1627@brightrain.aerifal.cx>
Date: Thu, 1 Jun 2017 12:16:38 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Use-after-free in __unlock
On Thu, Jun 01, 2017 at 11:57:53AM -0400, Rich Felker wrote:
> Thanks for finding and reporting this. Indeed, __lock and __unlock are
> not made safe for use on dynamic-lifetime objects, and I was wrongly
> thinking they were only used for static-lifetime ones (various
> libc-internal locks).
>
> For infrequently-used locks, which these seem to be, I see no reason
> to optimize for contention by having a separate waiter count; simply
> having a single atomic int whose states are "unlocked", "locked with
> no waiters", and "locked maybe with waiters" is a simple solution and
> slightly shrinks the relevant structures anyway. It's possible that
> this is the right solution for all places where __lock and __unlock
> are used, but we should probably do a review and see which ones might
> reasonably be subject to high contention (where spurious FUTEX_WAKE is
> very costly).
Quick analysis (based on search for ./-> inside __unlock/UNLOCK args):
Places where __lock/LOCK is used incorrectly:
- pthread struct
- DIR objects (maybe)
And places where it's used correctly:
- tz
- random
- syslog
- static-linked no-free version of malloc
- gettext
- locale
- pthread_atfork
- sem_open
- synccall
- atexit & variants
- open file list
Of the latter, these seem like things you could reasonably hammer
where false contention would be costly:
- tz
- random
- gettext
There are others where you could hammer, but they already involve much
heavier syscalls than futex so it's unlikely to matter what lock
contention tracking method is used.
I actually don't think the current "incorrect" use in DIR objects
matters, since there's no way to observe that thread B can legally
close a dir until _after_ thread A's call that took the lock returns
and thread A synchronizes with thread B. That is, mere use of
__lock/__unlock in dynamic-storage objects is not actually wrong; the
problem is specific to pthread exit lock and the related logic
concerning object lifetime. So a local fix may actually be more
appropriate.
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.
