The branch main has been updated by emaste:
URL:
https://cgit.FreeBSD.org/src/commit/?id=62f40433ab47ad4a9694a22a0313d57661502ca1
commit 62f40433ab47ad4a9694a22a0313d57661502ca1
Author: Olivier Certner <o...@freebsd.org>
AuthorDate: 2024-09-04 14:38:12 +0000
Commit: Ed Maste <ema...@freebsd.org>
CommitDate: 2024-09-04 14:38:12 +0000
umtx: shm: Fix use-after-free due to multiple drops of the registry
reference
umtx_shm_unref_reg_locked() would unconditionally drop the "registry"
reference, tied to USHMF_LINKED.
This is not a problem for caller umtx_shm_object_terminated(), which
operates under the 'umtx_shm_lock' lock end-to-end, but it is for
indirect caller umtx_shm(), which drops the lock between
umtx_shm_find_reg() and the call to umtx_shm_unref_reg(true) that
deregisters the umtx shared region (from 'umtx_shm_registry';
umtx_shm_find_reg() only finds registered shared mutexes).
Thus, two concurrent user-space callers of _umtx_op() with UMTX_OP_SHM
and flags UMTX_SHM_DESTROY, both progressing past umtx_shm_find_reg()
but before umtx_shm_unref_reg(true), would then decrease twice the
reference count for the single reference standing for the shared mutex's
registration.
Reported by: Synacktiv
Reviewed by: kib
Approved by: emaste (mentor)
Security: FreeBSD-SA-24:14.umtx
Security: CVE-2024-43102
Security: CAP-01
Sponsored by: The Alpha-Omega Project
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46126
---
sys/kern/kern_umtx.c | 51 +++++++++++++++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/sys/kern/kern_umtx.c b/sys/kern/kern_umtx.c
index 8d70438ea195..37c44f969a27 100644
--- a/sys/kern/kern_umtx.c
+++ b/sys/kern/kern_umtx.c
@@ -4384,39 +4384,49 @@ umtx_shm_free_reg(struct umtx_shm_reg *reg)
}
static bool
-umtx_shm_unref_reg_locked(struct umtx_shm_reg *reg, bool force)
+umtx_shm_unref_reg_locked(struct umtx_shm_reg *reg, bool linked_ref)
{
- bool res;
-
mtx_assert(&umtx_shm_lock, MA_OWNED);
KASSERT(reg->ushm_refcnt > 0, ("ushm_reg %p refcnt 0", reg));
- reg->ushm_refcnt--;
- res = reg->ushm_refcnt == 0;
- if (res || force) {
- if ((reg->ushm_flags & USHMF_LINKED) != 0) {
- TAILQ_REMOVE(&umtx_shm_registry[reg->ushm_key.hash],
- reg, ushm_reg_link);
- LIST_REMOVE(reg, ushm_obj_link);
- reg->ushm_flags &= ~USHMF_LINKED;
- }
+
+ if (linked_ref) {
+ if ((reg->ushm_flags & USHMF_LINKED) == 0)
+ /*
+ * The reference tied to USHMF_LINKED has already been
+ * released concurrently.
+ */
+ return (false);
+
+ TAILQ_REMOVE(&umtx_shm_registry[reg->ushm_key.hash], reg,
+ ushm_reg_link);
+ LIST_REMOVE(reg, ushm_obj_link);
+ reg->ushm_flags &= ~USHMF_LINKED;
}
- return (res);
+
+ reg->ushm_refcnt--;
+ return (reg->ushm_refcnt == 0);
}
static void
-umtx_shm_unref_reg(struct umtx_shm_reg *reg, bool force)
+umtx_shm_unref_reg(struct umtx_shm_reg *reg, bool linked_ref)
{
vm_object_t object;
bool dofree;
- if (force) {
+ if (linked_ref) {
+ /*
+ * Note: This may be executed multiple times on the same
+ * shared-memory VM object in presence of concurrent callers
+ * because 'umtx_shm_lock' is not held all along in umtx_shm()
+ * and here.
+ */
object = reg->ushm_obj->shm_object;
VM_OBJECT_WLOCK(object);
vm_object_set_flag(object, OBJ_UMTXDEAD);
VM_OBJECT_WUNLOCK(object);
}
mtx_lock(&umtx_shm_lock);
- dofree = umtx_shm_unref_reg_locked(reg, force);
+ dofree = umtx_shm_unref_reg_locked(reg, linked_ref);
mtx_unlock(&umtx_shm_lock);
if (dofree)
umtx_shm_free_reg(reg);
@@ -4469,7 +4479,6 @@ umtx_shm_create_reg(struct thread *td, const struct
umtx_key *key,
if (!chgumtxcnt(cred->cr_ruidinfo, 1, lim_cur(td, RLIMIT_UMTXP)))
return (ENOMEM);
reg = uma_zalloc(umtx_shm_reg_zone, M_WAITOK | M_ZERO);
- reg->ushm_refcnt = 1;
bcopy(key, ®->ushm_key, sizeof(*key));
reg->ushm_obj = shm_alloc(td->td_ucred, O_RDWR, false);
reg->ushm_cred = crhold(cred);
@@ -4486,11 +4495,17 @@ umtx_shm_create_reg(struct thread *td, const struct
umtx_key *key,
*res = reg1;
return (0);
}
- reg->ushm_refcnt++;
TAILQ_INSERT_TAIL(&umtx_shm_registry[key->hash], reg, ushm_reg_link);
LIST_INSERT_HEAD(USHM_OBJ_UMTX(key->info.shared.object), reg,
ushm_obj_link);
reg->ushm_flags = USHMF_LINKED;
+ /*
+ * This is one reference for the registry and the list of shared
+ * mutexes referenced by the VM object containing the lock pointer, and
+ * another for the caller, which it will free after use. So, one of
+ * these is tied to the presence of USHMF_LINKED.
+ */
+ reg->ushm_refcnt = 2;
mtx_unlock(&umtx_shm_lock);
*res = reg;
return (0);
