summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorManfred Spraul <manfred@colorfullife.com>2011-11-02 13:38:50 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2011-11-02 16:07:01 -0700
commit3c24783bb2deafaa106b7e69a97540071afc590c (patch)
tree904d59c7e197d59c472e20fc42057a5acb90f223
parent46cbc1d3981ee753518fbf9198a14f71a9f6841e (diff)
downloadlinux-3c24783bb2deafaa106b7e69a97540071afc590c.tar.bz2
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.c1
1 files changed, 0 insertions, 1 deletions
diff --git a/ipc/sem.c b/ipc/sem.c
index c8e00f8b4be1..fb13be17945b 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -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;
}