diff options
author | Manfred Spraul <manfred@colorfullife.com> | 2011-11-02 13:38:50 -0700 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2011-11-02 16:07:01 -0700 |
commit | 3c24783bb2deafaa106b7e69a97540071afc590c (patch) | |
tree | 904d59c7e197d59c472e20fc42057a5acb90f223 | |
parent | 46cbc1d3981ee753518fbf9198a14f71a9f6841e (diff) | |
download | op-kernel-dev-3c24783bb2deafaa106b7e69a97540071afc590c.zip op-kernel-dev-3c24783bb2deafaa106b7e69a97540071afc590c.tar.gz |
ipc/sem.c: fix return code race with semop vs. semop +semctl(IPC_RMID)
sys_semtimedop() may return -EIDRM although the semaphore operation
completed successfully:
thread 1: thread 2:
semtimedop(), sleeps
semop():
* acquires sem_lock()
semtimedop() woken up due to timeout
sem_lock() loops
* notices that thread 2 could be completed.
* performs the operations that thread 2 is sleeping on.
* marks the semaphore operation as IN_WAKEUP
* drops sem_lock(), does wakeup, sets return code to 0
* thread delayed due to interrupt, whatever
* returns to user space
* thread still delayed
semctl(IPC_RMID)
* acquires sem_lock()
* ipc_rmid(), ipcp->deleted=1
* drops sem_lock()
* thread finally continues - but seem_lock()
now fails due to ipcp->deleted == 1
* returns -EIDRM instead of 0
The fix is trivial: Always use the return code in queue.status.
In real world, the race probably doesn't matter:
If the semaphore array is destroyed, the app is probably not interested
if the last operation succeeded or was already cancelled.
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Galbraith <efault@gmx.de>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | ipc/sem.c | 1 |
1 files changed, 0 insertions, 1 deletions
@@ -1460,7 +1460,6 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, * Array removed? If yes, leave without sem_unlock(). */ if (IS_ERR(sma)) { - error = -EIDRM; goto out_free; } |