ipc: fix races with kern_ipc_perm.id and .seq

ipc_addid() initializes kern_ipc_perm.id and kern_ipc_perm.seq after
having called ipc_idr_alloc().

Thus a parallel semop() or msgrcv() that uses ipc_obtain_object_idr()
may see an uninitialized value.

The simple solution cannot be used, as the correct id is only known
after ipc_idr_alloc().

Therefore:
- Initialize kern_ipc_perm.seq to an invalid value, so that
  ipc_checkid() is guaranteed to fail.
  This fulfills the purpose of the sequence counter: If e.g. semget() and
  semop() run in parallel, then the semop() should not write into the
  newly created array.
- Move the accesses to kern_ipc_perm.id into the code that is protected
  by kern_ipc_perm.lock.

The patch also fixes a use-after free that can be triggered by concurrent
semget() and semctl(IPC_RMID): reading kern_ipc_perm.id must happen
before dropping the locks.

Reported-by: syzbot+2827ef6b3385deb07eaf@syzkaller.appspotmail.com
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>

ipc/msg.c

@@ -166,10 +166,12 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
 		return retval;
 	}
 
+	retval = msq->q_perm.id;
+
 	ipc_unlock_object(&msq->q_perm);
 	rcu_read_unlock();
 
-	return msq->q_perm.id;
+	return retval;
 }
 
 static inline bool msg_fits_inqueue(struct msg_queue *msq, size_t msgsz)
@@ -491,7 +493,6 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
 			 int cmd, struct msqid64_ds *p)
 {
 	struct msg_queue *msq;
-	int id = 0;
 	int err;
 
 	memset(p, 0, sizeof(*p));
@@ -503,7 +504,6 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
 			err = PTR_ERR(msq);
 			goto out_unlock;
 		}
-		id = msq->q_perm.id;
 	} else { /* IPC_STAT */
 		msq = msq_obtain_object_check(ns, msqid);
 		if (IS_ERR(msq)) {
@@ -548,10 +548,21 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
 	p->msg_lspid  = pid_vnr(msq->q_lspid);
 	p->msg_lrpid  = pid_vnr(msq->q_lrpid);
 
-	ipc_unlock_object(&msq->q_perm);
-	rcu_read_unlock();
-	return id;
+	if (cmd == IPC_STAT) {
+		/*
+		 * As defined in SUS:
+		 * Return 0 on success
+		 */
+		err = 0;
+	} else {
+		/*
+		 * MSG_STAT and MSG_STAT_ANY (both Linux specific)
+		 * Return the full id, including the sequence counter
+		 */
+		err = msq->q_perm.id;
+	}
 
+	ipc_unlock_object(&msq->q_perm);
 out_unlock:
 	rcu_read_unlock();
 	return err;

ipc/sem.c

@@ -561,13 +561,14 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 	}
 	ns->used_sems += nsems;
 
+	retval = sma->sem_perm.id;
+
 	sem_unlock(sma, -1);
 	rcu_read_unlock();
 
-	return sma->sem_perm.id;
+	return retval;
 }
 
-
 /*
  * Called with sem_ids.rwsem and ipcp locked.
  */
@@ -1222,7 +1223,6 @@ static int semctl_stat(struct ipc_namespace *ns, int semid,
 {
 	struct sem_array *sma;
 	time64_t semotime;
-	int id = 0;
 	int err;
 
 	memset(semid64, 0, sizeof(*semid64));
@@ -1234,7 +1234,6 @@ static int semctl_stat(struct ipc_namespace *ns, int semid,
 			err = PTR_ERR(sma);
 			goto out_unlock;
 		}
-		id = sma->sem_perm.id;
 	} else { /* IPC_STAT */
 		sma = sem_obtain_object_check(ns, semid);
 		if (IS_ERR(sma)) {
@@ -1274,10 +1273,20 @@ static int semctl_stat(struct ipc_namespace *ns, int semid,
 #endif
 	semid64->sem_nsems = sma->sem_nsems;
 
+	if (cmd == IPC_STAT) {
+		/*
+		 * As defined in SUS:
+		 * Return 0 on success
+		 */
+		err = 0;
+	} else {
+		/*
+		 * SEM_STAT and SEM_STAT_ANY (both Linux specific)
+		 * Return the full id, including the sequence counter
+		 */
+		err = sma->sem_perm.id;
+	}
 	ipc_unlock_object(&sma->sem_perm);
-	rcu_read_unlock();
-	return id;
-
 out_unlock:
 	rcu_read_unlock();
 	return err;

ipc/shm.c

@@ -949,7 +949,6 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
 			int cmd, struct shmid64_ds *tbuf)
 {
 	struct shmid_kernel *shp;
-	int id = 0;
 	int err;
 
 	memset(tbuf, 0, sizeof(*tbuf));
@@ -961,7 +960,6 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
 			err = PTR_ERR(shp);
 			goto out_unlock;
 		}
-		id = shp->shm_perm.id;
 	} else { /* IPC_STAT */
 		shp = shm_obtain_object_check(ns, shmid);
 		if (IS_ERR(shp)) {
@@ -1011,10 +1009,21 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
 	tbuf->shm_lpid	= pid_vnr(shp->shm_lprid);
 	tbuf->shm_nattch = shp->shm_nattch;
 
-	ipc_unlock_object(&shp->shm_perm);
-	rcu_read_unlock();
-	return id;
+	if (cmd == IPC_STAT) {
+		/*
+		 * As defined in SUS:
+		 * Return 0 on success
+		 */
+		err = 0;
+	} else {
+		/*
+		 * SHM_STAT and SHM_STAT_ANY (both Linux specific)
+		 * Return the full id, including the sequence counter
+		 */
+		err = shp->shm_perm.id;
+	}
 
+	ipc_unlock_object(&shp->shm_perm);
 out_unlock:
 	rcu_read_unlock();
 	return err;

ipc/util.c

@@ -22,7 +22,7 @@
  *          obtain the ipc object (kern_ipc_perm) by looking up the id in an idr
  *	    tree.
  *	    - perform initial checks (capabilities, auditing and permission,
- *	      etc).
+ *	      check the sequence counter, etc).
  *	    - perform read-only operations, such as INFO command, that
  *	      do not demand atomicity
  *	      acquire the ipc lock (kern_ipc_perm.lock) through
@@ -270,6 +270,12 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
 	new->cuid = new->uid = euid;
 	new->gid = new->cgid = egid;
 
+	/*
+	 * Initialize ->seq to ULONG_MAX, so that any early ipc_checkid()
+	 * calls are guarenteed to fail.
+	 */
+	new->seq = ULONG_MAX;
+
 	id = ipc_idr_alloc(ids, new);
 	idr_preload_end();