Commit 0f3d2b01 authored by Rafael Aquini's avatar Rafael Aquini Committed by Linus Torvalds

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: default avatarRafael Aquini <aquini@redhat.com>
Acked-by: default avatarRik van Riel <riel@redhat.com>
Acked-by: default avatarGreg Thelen <gthelen@google.com>
Reviewed-by: default avatarDavidlohr Bueso <davidlohr@hp.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 78f5009c
...@@ -696,7 +696,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext, ...@@ -696,7 +696,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
goto out_unlock0; goto out_unlock0;
/* raced with RMID? */ /* raced with RMID? */
if (msq->q_perm.deleted) { if (!ipc_valid_object(&msq->q_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock0; goto out_unlock0;
} }
...@@ -731,7 +731,8 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext, ...@@ -731,7 +731,8 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
ipc_lock_object(&msq->q_perm); ipc_lock_object(&msq->q_perm);
ipc_rcu_putref(msq, ipc_rcu_free); ipc_rcu_putref(msq, ipc_rcu_free);
if (msq->q_perm.deleted) { /* raced with RMID? */
if (!ipc_valid_object(&msq->q_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock0; goto out_unlock0;
} }
...@@ -909,7 +910,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl ...@@ -909,7 +910,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
ipc_lock_object(&msq->q_perm); ipc_lock_object(&msq->q_perm);
/* raced with RMID? */ /* raced with RMID? */
if (msq->q_perm.deleted) { if (!ipc_valid_object(&msq->q_perm)) {
msg = ERR_PTR(-EIDRM); msg = ERR_PTR(-EIDRM);
goto out_unlock0; goto out_unlock0;
} }
......
...@@ -1284,7 +1284,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1284,7 +1284,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum,
sem_lock(sma, NULL, -1); sem_lock(sma, NULL, -1);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
sem_unlock(sma, -1); sem_unlock(sma, -1);
rcu_read_unlock(); rcu_read_unlock();
return -EIDRM; return -EIDRM;
...@@ -1344,7 +1344,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1344,7 +1344,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
int i; int i;
sem_lock(sma, NULL, -1); sem_lock(sma, NULL, -1);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock; goto out_unlock;
} }
...@@ -1363,7 +1363,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1363,7 +1363,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
rcu_read_lock(); rcu_read_lock();
sem_lock_and_putref(sma); sem_lock_and_putref(sma);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock; goto out_unlock;
} }
...@@ -1411,7 +1411,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1411,7 +1411,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
} }
rcu_read_lock(); rcu_read_lock();
sem_lock_and_putref(sma); sem_lock_and_putref(sma);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock; goto out_unlock;
} }
...@@ -1437,7 +1437,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1437,7 +1437,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
goto out_rcu_wakeup; goto out_rcu_wakeup;
sem_lock(sma, NULL, -1); sem_lock(sma, NULL, -1);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
err = -EIDRM; err = -EIDRM;
goto out_unlock; goto out_unlock;
} }
...@@ -1701,7 +1701,7 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid) ...@@ -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 */ /* step 3: Acquire the lock on semaphore array */
rcu_read_lock(); rcu_read_lock();
sem_lock_and_putref(sma); sem_lock_and_putref(sma);
if (sma->sem_perm.deleted) { if (!ipc_valid_object(&sma->sem_perm)) {
sem_unlock(sma, -1); sem_unlock(sma, -1);
rcu_read_unlock(); rcu_read_unlock();
kfree(new); kfree(new);
...@@ -1848,7 +1848,15 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1848,7 +1848,15 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
error = -EIDRM; error = -EIDRM;
locknum = sem_lock(sma, sops, nsops); 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; goto out_unlock_free;
/* /*
* semid identifiers are not unique - find_alloc_undo may have * semid identifiers are not unique - find_alloc_undo may have
...@@ -2070,7 +2078,7 @@ void exit_sem(struct task_struct *tsk) ...@@ -2070,7 +2078,7 @@ void exit_sem(struct task_struct *tsk)
sem_lock(sma, NULL, -1); sem_lock(sma, NULL, -1);
/* exit_sem raced with IPC_RMID, nothing to do */ /* 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); sem_unlock(sma, -1);
rcu_read_unlock(); rcu_read_unlock();
continue; continue;
......
...@@ -975,6 +975,13 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf) ...@@ -975,6 +975,13 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf)
goto out_unlock1; goto out_unlock1;
ipc_lock_object(&shp->shm_perm); ipc_lock_object(&shp->shm_perm);
/* check if shm_destroy() is tearing down shp */
if (!ipc_valid_object(&shp->shm_perm)) {
err = -EIDRM;
goto out_unlock0;
}
if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) { if (!ns_capable(ns->user_ns, CAP_IPC_LOCK)) {
kuid_t euid = current_euid(); kuid_t euid = current_euid();
if (!uid_eq(euid, shp->shm_perm.uid) && if (!uid_eq(euid, shp->shm_perm.uid) &&
...@@ -989,13 +996,6 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf) ...@@ -989,13 +996,6 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf)
} }
shm_file = shp->shm_file; shm_file = shp->shm_file;
/* check if shm_destroy() is tearing down shp */
if (shm_file == NULL) {
err = -EIDRM;
goto out_unlock0;
}
if (is_file_hugepages(shm_file)) if (is_file_hugepages(shm_file))
goto out_unlock0; goto out_unlock0;
...@@ -1116,7 +1116,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr, ...@@ -1116,7 +1116,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr,
ipc_lock_object(&shp->shm_perm); ipc_lock_object(&shp->shm_perm);
/* check if shm_destroy() is tearing down shp */ /* check if shm_destroy() is tearing down shp */
if (shp->shm_file == NULL) { if (!ipc_valid_object(&shp->shm_perm)) {
ipc_unlock_object(&shp->shm_perm); ipc_unlock_object(&shp->shm_perm);
err = -EIDRM; err = -EIDRM;
goto out_unlock; goto out_unlock;
......
...@@ -185,6 +185,19 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm) ...@@ -185,6 +185,19 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm)
rcu_read_unlock(); rcu_read_unlock();
} }
/*
* ipc_valid_object() - helper to sort out IPC_RMID races for codepaths
* where the respective ipc_ids.rwsem is not being held down.
* Checks whether the ipc object is still around or if it's gone already, as
* ipc_rmid() may have already freed the ID while the ipc lock was spinning.
* Needs to be called with kern_ipc_perm.lock held -- exception made for one
* checkpoint case at sys_semtimedop() as noted in code commentary.
*/
static inline bool ipc_valid_object(struct kern_ipc_perm *perm)
{
return perm->deleted == 0;
}
struct kern_ipc_perm *ipc_obtain_object_check(struct ipc_ids *ids, int id); struct kern_ipc_perm *ipc_obtain_object_check(struct ipc_ids *ids, int id);
int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids, int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
struct ipc_ops *ops, struct ipc_params *params); struct ipc_ops *ops, struct ipc_params *params);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment