Date: Wed, 29 Jul 2026 17:47:59 +0000
From: Mark Johnston <markj@FreeBSD.org>
To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject: git: 8c5e2d26cde5 - main - sysvsem: Fix a TOCTOU race in semctl({GET,SET}ALL)
Message-ID: <6a6a3ccf.397b8.175f93b4@gitrepo.freebsd.org>
index | next in thread | raw e-mail
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",home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6a3ccf.397b8.175f93b4>