Skip to content

Commit b52be55

Browse files
thejhtorvalds
authored andcommitted
ipc/sem: Fix dangling sem_array access in semtimedop race
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: 370b262 ("ipc/sem: avoid idr tree lookup for interrupted semop") Cc: [email protected] Signed-off-by: Jann Horn <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 76dcd73 commit b52be55

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

ipc/sem.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2179,14 +2179,15 @@ long __do_semtimedop(int semid, struct sembuf *sops,
21792179
* scenarios where we were awakened externally, during the
21802180
* window between wake_q_add() and wake_up_q().
21812181
*/
2182+
rcu_read_lock();
21822183
error = READ_ONCE(queue.status);
21832184
if (error != -EINTR) {
21842185
/* see SEM_BARRIER_2 for purpose/pairing */
21852186
smp_acquire__after_ctrl_dep();
2187+
rcu_read_unlock();
21862188
goto out;
21872189
}
21882190

2189-
rcu_read_lock();
21902191
locknum = sem_lock(sma, sops, nsops);
21912192

21922193
if (!ipc_valid_object(&sma->sem_perm))

0 commit comments

Comments
 (0)