Re: [PATCH v2] ipc: introduce ipc_valid_object() helper to sort outIPC_RMID races

From: Manfred Spraul
Date: Wed Dec 18 2013 - 14:00:46 EST


Hi Rafael,

On 12/18/2013 06:34 PM, Rafael Aquini wrote:
Folks,

Before I re-submit the v3 with the commentary changes requested, I'm pasting
here what I'm planning to amend to v2 patch:
---
diff --git a/ipc/sem.c b/ipc/sem.c
index ed0057a..23379b6 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1846,6 +1846,14 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __u
error = -EIDRM;
locknum = sem_lock(sma, sops, nsops);
+ /*
+ * We eventually might perform the following check in a lockless
+ * fashion here, considering ipc_valid_object() locking constraints.
+ * If nsops == 1 and there's no contention for sem_perm.lock, then
+ * only a per-semaphore lock is held and it's OK to go on 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;
/*
diff --git a/ipc/util.h b/ipc/util.h
index 071ed58..d05b708 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -190,7 +190,8 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm)
* 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.
+ * 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)
{
---

Do we need to change somthing else?
Looking forward your thoughts!
Acked-by: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>

--
Manfred
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/