[PATCH] ipc/sem.c: ensure proper locking during namespace teardown

[Manfred Spraul]

free_ipcs() only calls ipc_lock_object() before calling the free callback.

This means:
- There is no exclusion against parallel simple semop() calls.
- sma->use_global_lock may underflow (i.e. jump to UNIT_MAX) when
  freeary() calls sem_unlock(,,-1).

The patch fixes that, by adding complexmode_enter() before calling
freeary().

There are multiple syzbot crashes in this code area, but I don't see yet
how a missing complexmode_enter() may cause a crash:
- 1) simple semop() calls are not used by these syzbox tests,
  and 2) we are in namespace teardown, noone may run in parallel.

- 1) freeary() is the last call (except parallel operations, which
  are impossible due to namespace teardown)
  and 2) the underflow of use_global_lock merely delays switching to
  parallel simple semop handling for the next UINT_MAX semop() calls.

Thus I think the patch is "only" a cleanup, and does not fix
the observed crashes.

Signed-off-by: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
Reported-by: syzbot+1145ec2e23165570c3ac@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+c92d3646e35bc5d1a909@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: syzbot+9d8b6fa6ee7636f350c1@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: dvyukov@xxxxxxxxxx
Cc: dbueso@xxxxxxx
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
 ipc/sem.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 745dc6187e84..8ccacd11fb15 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -184,6 +184,9 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
  */
 #define USE_GLOBAL_LOCK_HYSTERESIS	10
 
+static void complexmode_enter(struct sem_array *sma);
+static void complexmode_tryleave(struct sem_array *sma);
+
 /*
  * Locking:
  * a) global sem_lock() for read/write
@@ -232,9 +235,24 @@ void sem_init_ns(struct ipc_namespace *ns)
 }
 
 #ifdef CONFIG_IPC_NS
+
+static void freeary_lock(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
+{
+	struct sem_array *sma = container_of(ipcp, struct sem_array, sem_perm);
+
+	/*
+	 * free_ipcs() isn't aware of sem_lock(), it calls ipc_lock_object()
+	 * directly. In order to stay compatible with sem_lock(), we must
+	 * upgrade from "simple" ipc_lock_object() to sem_lock(,,-1).
+	 */
+	complexmode_enter(sma);
+
+	freeary(ns, ipcp);
+}
+
 void sem_exit_ns(struct ipc_namespace *ns)
 {
-	free_ipcs(ns, &sem_ids(ns), freeary);
+	free_ipcs(ns, &sem_ids(ns), freeary_lock);
 	idr_destroy(&ns->ids[IPC_SEM_IDS].ipcs_idr);
 	rhashtable_destroy(&ns->ids[IPC_SEM_IDS].key_ht);
 }
@@ -374,7 +392,9 @@ static inline int sem_lock(struct sem_array *sma, struct sembuf *sops,
 		/* Complex operation - acquire a full lock */
 		ipc_lock_object(&sma->sem_perm);
 
-		/* Prevent parallel simple ops */
+		/* Prevent parallel simple ops.
+		 * This must be identical to freeary_lock().
+		 */
 		complexmode_enter(sma);
 		return SEM_GLOBAL_LOCK;
 	}
-- 
2.17.2


--------------348420D97650A12C69C239A1--