From 0f3d2b0135f4bdbfe47a99753923a64efd373d11 Mon Sep 17 00:00:00 2001 From: Rafael Aquini Date: Mon, 27 Jan 2014 17:07:01 -0800 Subject: ipc: introduce ipc_valid_object() helper to sort out IPC_RMID races After the locking semantics for the SysV IPC API got improved, a couple of IPC_RMID race windows were opened because we ended up dropping the 'kern_ipc_perm.deleted' check performed way down in ipc_lock(). The spotted races got sorted out by re-introducing the old test within the racy critical sections. This patch introduces ipc_valid_object() to consolidate the way we cope with IPC_RMID races by using the same abstraction across the API implementation. Signed-off-by: Rafael Aquini Acked-by: Rik van Riel Acked-by: Greg Thelen Reviewed-by: Davidlohr Bueso Cc: Manfred Spraul Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- ipc/sem.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'ipc/sem.c') diff --git a/ipc/sem.c b/ipc/sem.c index cc9ac35b793c..4d88194a5ffe 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -1284,7 +1284,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum, sem_lock(sma, NULL, -1); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { sem_unlock(sma, -1); rcu_read_unlock(); return -EIDRM; @@ -1344,7 +1344,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, int i; sem_lock(sma, NULL, -1); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { err = -EIDRM; goto out_unlock; } @@ -1363,7 +1363,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, rcu_read_lock(); sem_lock_and_putref(sma); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { err = -EIDRM; goto out_unlock; } @@ -1411,7 +1411,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, } rcu_read_lock(); sem_lock_and_putref(sma); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { err = -EIDRM; goto out_unlock; } @@ -1437,7 +1437,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, goto out_rcu_wakeup; sem_lock(sma, NULL, -1); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { err = -EIDRM; goto out_unlock; } @@ -1701,7 +1701,7 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid) /* step 3: Acquire the lock on semaphore array */ rcu_read_lock(); sem_lock_and_putref(sma); - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { sem_unlock(sma, -1); rcu_read_unlock(); kfree(new); @@ -1848,7 +1848,15 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, error = -EIDRM; locknum = sem_lock(sma, sops, nsops); - if (sma->sem_perm.deleted) + /* + * We eventually might perform the following check in a lockless + * fashion, considering ipc_valid_object() locking constraints. + * If nsops == 1 and there is no contention for sem_perm.lock, then + * only a per-semaphore lock is held and it's OK to proceed with the + * check below. More details on the fine grained locking scheme + * entangled here and why it's RMID race safe on comments at sem_lock() + */ + if (!ipc_valid_object(&sma->sem_perm)) goto out_unlock_free; /* * semid identifiers are not unique - find_alloc_undo may have @@ -2070,7 +2078,7 @@ void exit_sem(struct task_struct *tsk) sem_lock(sma, NULL, -1); /* exit_sem raced with IPC_RMID, nothing to do */ - if (sma->sem_perm.deleted) { + if (!ipc_valid_object(&sma->sem_perm)) { sem_unlock(sma, -1); rcu_read_unlock(); continue; -- cgit v1.2.3-59-g8ed1b