diff options
| author | Jann Horn <jannh@google.com> | 2022-12-05 17:59:27 +0100 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2022-12-08 11:16:33 +0100 |
| commit | 39a60b24d1d7c3509d2ff9768ec4ca6c292419cb (patch) | |
| tree | f39926e7e3f0f607760a70f4083992e96d38e242 /ipc | |
| parent | 72d35aa86e4ce6f90b122ad62b37722f4d2a07df (diff) | |
| download | linux-39a60b24d1d7c3509d2ff9768ec4ca6c292419cb.tar.gz linux-39a60b24d1d7c3509d2ff9768ec4ca6c292419cb.tar.bz2 linux-39a60b24d1d7c3509d2ff9768ec4ca6c292419cb.zip | |
ipc/sem: Fix dangling sem_array access in semtimedop race
[ Upstream commit b52be557e24c47286738276121177a41f54e3b83 ]
When __do_semtimedop() goes to sleep because it has to wait for a
semaphore value becoming zero or becoming bigger than some threshold, it
links the on-stack sem_queue to the sem_array, then goes to sleep
without holding a reference on the sem_array.
When __do_semtimedop() comes back out of sleep, one of two things must
happen:
a) We prove that the on-stack sem_queue has been disconnected from the
(possibly freed) sem_array, making it safe to return from the stack
frame that the sem_queue exists in.
b) We stabilize our reference to the sem_array, lock the sem_array, and
detach the sem_queue from the sem_array ourselves.
sem_array has RCU lifetime, so for case (b), the reference can be
stabilized inside an RCU read-side critical section by locklessly
checking whether the sem_queue is still connected to the sem_array.
However, the current code does the lockless check on sem_queue before
starting an RCU read-side critical section, so the result of the
lockless check immediately becomes useless.
Fix it by doing rcu_read_lock() before the lockless check. Now RCU
ensures that if we observe the object being on our queue, the object
can't be freed until rcu_read_unlock().
This bug is only hittable on kernel builds with full preemption support
(either CONFIG_PREEMPT or PREEMPT_DYNAMIC with preempt=full).
Fixes: 370b262c896e ("ipc/sem: avoid idr tree lookup for interrupted semop")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Diffstat (limited to 'ipc')
| -rw-r--r-- | ipc/sem.c | 3 |
1 files changed, 2 insertions, 1 deletions
diff --git a/ipc/sem.c b/ipc/sem.c index 6adc245f3e02..89fef4550b38 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -2064,6 +2064,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops, * scenarios where we were awakened externally, during the * window between wake_q_add() and wake_up_q(). */ + rcu_read_lock(); error = READ_ONCE(queue.status); if (error != -EINTR) { /* @@ -2073,10 +2074,10 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops, * overwritten by the previous owner of the semaphore. */ smp_mb(); + rcu_read_unlock(); goto out_free; } - rcu_read_lock(); locknum = sem_lock(sma, sops, nsops); if (!ipc_valid_object(&sma->sem_perm)) |