git: 8c5e2d26cde5 - main - sysvsem: Fix a TOCTOU race in semctl({GET,SET}ALL)

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=8c5e2d26cde5d06c362e6925dbcdbd43aa315a12

commit 8c5e2d26cde5d06c362e6925dbcdbd43aa315a12
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-07-27 15:41:30 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-07-29 17:45:02 +0000

    sysvsem: Fix a TOCTOU race in semctl({GET,SET}ALL)
    
    These commands take a snapshot of the size of a semaphore set, then drop
    the lock and malloc an appropriately sized array before reacquiring the
    lock.  A comment explains why this is (probably) safe.  Unfortunately,
    it's wrong; it is indeed possible for a malicious userspace to create
    and destroy 2^{15} sets in the window where the lock is dropped.  This
    race can lead to out-of-bounds reads and writes, and that can be
    exploited to elevate privileges.
    
    Replace the assertions with runtime checks.
    
    Approved by:    so
    Security:       FreeBSD-SA-26:54.sysvsem
    Security:       CVE-2026-58087
    Reported by:    Maik Muench of Secfault Security
    Reviewed by:    kib
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D58421
---
 sys/kern/sysv_sem.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/sys/kern/sysv_sem.c b/sys/kern/sysv_sem.c
index a99e1a4de14e..8c3dc6c0d9ed 100644
--- a/sys/kern/sysv_sem.c
+++ b/sys/kern/sysv_sem.c
@@ -848,25 +848,20 @@ kern_semctl(struct thread *td, int semid, int semnum, int cmd,
 		 * won't work for SETALL since we can't copyin() more
 		 * data than the user specified as we may return a
 		 * spurious EFAULT.
-		 *
-		 * Note that the number of semaphores in a set is
-		 * fixed for the life of that set.  The only way that
-		 * the 'count' could change while are blocked in
-		 * malloc() is if this semaphore set were destroyed
-		 * and a new one created with the same index.
-		 * However, semvalid() will catch that due to the
-		 * sequence number unless exactly 0x8000 (or a
-		 * multiple thereof) semaphore sets for the same index
-		 * are created and destroyed while we are in malloc!
-		 *
 		 */
+		if ((error = semvalid(semid, rpr, semakptr)) != 0)
+			goto done2;
 		count = semakptr->u.sem_nsems;
 		mtx_unlock(sema_mtxp);
 		array = malloc(sizeof(*array) * count, M_TEMP, M_WAITOK);
 		mtx_lock(sema_mtxp);
 		if ((error = semvalid(semid, rpr, semakptr)) != 0)
 			goto done2;
-		KASSERT(count == semakptr->u.sem_nsems, ("nsems changed"));
+		if (count != semakptr->u.sem_nsems) {
+			/* Unlikely, but possible. */
+			error = EAGAIN;
+			goto done2;
+		}
 		if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R)))
 			goto done2;
 		for (i = 0; i < semakptr->u.sem_nsems; i++)
@@ -909,10 +904,8 @@ kern_semctl(struct thread *td, int semid, int semnum, int cmd,
 		break;
 
 	case SETALL:
-		/*
-		 * See comment on GETALL for why 'count' shouldn't change
-		 * and why we require a userland buffer.
-		 */
+		if ((error = semvalid(semid, rpr, semakptr)) != 0)
+			goto done2;
 		count = semakptr->u.sem_nsems;
 		mtx_unlock(sema_mtxp);
 		array = malloc(sizeof(*array) * count, M_TEMP, M_WAITOK);
@@ -922,7 +915,11 @@ kern_semctl(struct thread *td, int semid, int semnum, int cmd,
 			break;
 		if ((error = semvalid(semid, rpr, semakptr)) != 0)
 			goto done2;
-		KASSERT(count == semakptr->u.sem_nsems, ("nsems changed"));
+		if (count != semakptr->u.sem_nsems) {
+			/* Unlikely, but possible. */
+			error = EAGAIN;
+			goto done2;
+		}
 		if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_W)))
 			goto done2;
 		for (i = 0; i < semakptr->u.sem_nsems; i++) {
@@ -1482,12 +1479,11 @@ semexit_myhook(void *arg, struct proc *p)
 
 			mtx_lock(sema_mtxp);
 			if ((semakptr->u.sem_perm.mode & SEM_ALLOC) == 0 ||
-			    (semakptr->u.sem_perm.seq != seq)) {
+			    semakptr->u.sem_perm.seq != seq ||
+			    semakptr->u.sem_nsems <= semnum) {
 				mtx_unlock(sema_mtxp);
 				continue;
 			}
-			if (semnum >= semakptr->u.sem_nsems)
-				panic("semexit - semnum out of range");
 
 			DPRINTF((
 			    "semexit:  %p id=%d num=%d(adj=%d) ; sem=%d\n",