On 11/30/23 17:24, Jann Horn wrote:
On Thu, Nov 30, 2023 at 10:53 PM Waiman Long <longman@xxxxxxxxxx> wrote:
On 11/30/23 15:48, Jann Horn wrote:
I have seen several cases of attempts to use mutex_unlock() to release an
object such that the object can then be freed by another task.
My understanding is that this is not safe because mutex_unlock(), in the
MUTEX_FLAG_WAITERS && !MUTEX_FLAG_HANDOFF case, accesses the mutex
structure after having marked it as unlocked; so mutex_unlock() requires
its caller to ensure that the mutex stays alive until mutex_unlock()
returns.
If MUTEX_FLAG_WAITERS is set and there are real waiters, those waiters
have to keep the mutex alive, I think; but we could have a spurious
MUTEX_FLAG_WAITERS left if an interruptible/killable waiter bailed
between the points where __mutex_unlock_slowpath() did the cmpxchg
reading the flags and where it acquired the wait_lock.
Could you clarify under what condition a concurrent task can decide to
free the object holding the mutex? Is it !mutex_is_locked() or after a
mutex_lock()/mutex_unlock sequence?
I mean a mutex_lock()+mutex_unlock() sequence.
Because of optimistic spinning, a mutex_lock()/mutex_unlock() can
succeed even if there are still waiters waiting for the lock.
mutex_is_locked() will return true if the mutex has waiter even if it
is currently free.
I don't understand your point, and maybe I also don't understand what
you mean by "free". Isn't mutex_is_locked() defined such that it only
looks at whether a mutex has an owner, and doesn't look at the waiter
list?
What I mean is that the mutex is in an unlocked state ready to be
acquired by another locker. mutex_is_locked() considers the state of the
mutex as locked if any of the owner flags is set.
Beside the mutex_lock()/mutex_unlock() sequence, I will suggest adding a
mutex_is_locked() check just to be sure.
Cheers,
Longman
