Re: [PATCH] locking: Document that mutex_unlock() is non-atomic

From: Waiman Long
Date: Sat Dec 02 2023 - 17:39:47 EST


On 12/2/23 10:51, David Laight wrote:
From: Waiman Long
Sent: 01 December 2023 19:16

On 12/1/23 13:44, David Laight wrote:
Pending waiters aren't the problem.

Pending waiters can still be a problem if code decides to free the lock
containing object after a lock/unlock sequence as it may cause
use-after-free.
You have to ensure there aren't any, but the mutex() can be held.

Using reference count to track the number of active users is one way to
prevent that if you only release the reference count after
mutex_unlock() returns but not in the lock critical section.
I suspect the documentation need to be more explicit than just saying
it is non-atomic.
Saying something like:

The mutex structure may be accessed by mutex_unlock() after another
thread has locked and unlocked the mutex.

So if a reference count is used to ensure a structure remains valid when
a lock is released (with the item being freed when the count becomes zero)
the reference count itself cannot be protected by a mutex in the structure.
So code like:
...
count = --item->refcount;
mutex_unlock(item->mtx);
if (!count)
free(item);
can lead to a 'use after free' in mutex_unlock().
However if the refcount is atomic and decremented without the
mutex held there isn't a problem.

David

That is definitely better than saying it is non-atomic which is vague in meaning.

Cheers,
Longman