⬢ DragonFlyBSD Kernel Audit
DF-5212 / fix.diff
← back to finding ↓ download raw
# DF-5212 - cxm(4) REMOTE_GETKEY ioctl vs detach use-after-free (cxm_ir.c path)
#
# cxm_ir_key() (sys/dev/video/cxm/cxm_ir.c:100-117) can tsleep 50 ms
# (cxm_ir.c:114) inside cxm_ioctl's REMOTE_GETKEY case (cxm.c:2894-2898)
# with the softc referenced only from the sleeping thread's stack.
# cxm_detach() (cxm.c:1944-2004) tears down the iicbus children
# (cxm.c:1993-1996) and frees the softc without draining in-flight
# ioctls (dev_ops_remove_minor at cxm.c:1957 only dead-vectors NEW
# calls), so the sleeper resumes into freed memory.
#
# Fix: serialize the cdev ioctl entry against cxm_detach with a softc
# lock (exclusive in the entry wrapper, drained in detach before
# anything the ioctl body touches is freed). Same scheme as - and fully
# subsumed by - the pending DF-4753 fix, which wraps the whole
# cxm_ioctl switch; this is the standalone minimal subset covering the
# REMOTE_GETKEY / cxm_ir_key path.
#
# Compile-validated as the full cxm.ko module on the QEMU guest
# (fix_build.log, rc=0). Authored post-verification; never applied to
# the read-only sys/ tree.

--- a/sys/dev/video/cxm/cxm.h	2026-10-01 04:16:18.260794303 +0000
+++ b/sys/dev/video/cxm/cxm.h	2026-10-01 04:16:34.820582308 +0000
@@ -37,6 +37,7 @@
  */
 
 #include <sys/event.h>
+#include <sys/lock.h>
 
 #include "pcidevs.h"
 
@@ -254,6 +255,7 @@
 
 	device_t	dev;		/* bus attachment */
 	cdev_t		cxm_dev_t;	/* control device */
+	struct lock	cxm_lock;	/* serializes cdev ioctls vs detach */
 	int		is_opened;
 	int		mpeg;
 
--- a/sys/dev/video/cxm/cxm.c	2026-10-01 04:16:18.260794303 +0000
+++ b/sys/dev/video/cxm/cxm.c	2026-10-01 04:16:34.800582565 +0000
@@ -120,6 +120,9 @@
 static	d_close_t	cxm_close;
 static	d_read_t	cxm_read;
 static	d_ioctl_t	cxm_ioctl;
+
+static int cxm_ioctl_locked(struct dev_ioctl_args *, struct cxm_softc *);
+
 static	d_kqfilter_t	cxm_kqfilter;
 
 static void cxm_filter_detach(struct knote *);
@@ -1880,6 +1883,7 @@
 
 	sc->source = cxm_tuner_source;
 
+	lockinit(&sc->cxm_lock, "cxm", 0, 0);
 
 	/* make the device entries */
 	sc->cxm_dev_t = make_dev(&cxm_ops, unit,
@@ -1957,6 +1961,15 @@
 	dev_ops_remove_minor(&cxm_ops, /*0, */device_get_unit(dev));
 
 	/*
+	 * Drain any in-flight cdev operations (REMOTE_GETKEY can
+	 * sleep 50 ms in cxm_ir_key at cxm_ir.c:114 while
+	 * dereferencing sc) before tearing down the softc and the
+	 * iicbus they use.
+	 */
+	lockmgr(&sc->cxm_lock, LK_EXCLUSIVE);
+	lockmgr(&sc->cxm_lock, LK_RELEASE);
+
+	/*
 	 * Deallocate scatter / gather list and buffers.
 	 */
 	bus_dmamap_unload(sc->enc_sg.dmat, sc->enc_sg.dmamap);
@@ -2000,6 +2013,8 @@
 	bus_release_resource(dev, SYS_RES_IRQ, 0, sc->irq_res);
 	bus_release_resource(dev, SYS_RES_MEMORY, CXM_RID, sc->mem_res);
 
+	lockuninit(&sc->cxm_lock);
+
 	return 0;
 }
 
@@ -2305,7 +2320,29 @@
 static int
 cxm_ioctl(struct dev_ioctl_args *ap)
 {
-	cdev_t		dev = ap->a_head.a_dev;
+	struct cxm_softc *sc;
+	int		error;
+	int		unit;
+
+	unit = UNIT(minor(ap->a_head.a_dev));
+
+	/* Get the device data */
+	sc = (struct cxm_softc*)devclass_get_softc(cxm_devclass, unit);
+	if (sc == NULL) {
+		/* the device is no longer valid/functioning */
+		return ENXIO;
+	}
+
+	lockmgr(&sc->cxm_lock, LK_EXCLUSIVE);
+	error = cxm_ioctl_locked(ap, sc);
+	lockmgr(&sc->cxm_lock, LK_RELEASE);
+
+	return error;
+}
+
+static int
+cxm_ioctl_locked(struct dev_ioctl_args *ap, struct cxm_softc *sc)
+{
 	int		brightness;
 	int		chroma_saturation;
 	int		contrast;
@@ -2313,24 +2350,13 @@
 	int		hue;
 	int		result;
 	int		status;
-	int		unit;
 	unsigned int	i;
 	unsigned int	sig;
 	unsigned long	freq;
-	struct cxm_softc *sc;
 	enum cxm_source	source;
 	struct bktr_capture_area *cap;
 	struct bktr_remote *remote;
 
-	unit = UNIT(minor(dev));
-
-	/* Get the device data */
-	sc = (struct cxm_softc*)devclass_get_softc(cxm_devclass, unit);
-	if (sc == NULL) {
-		/* the device is no longer valid/functioning */
-		return ENXIO;
-	}
-
 	switch (ap->a_cmd) {
 	case BT848_GAUDIO:
 		switch (cxm_msp_selected_source(sc)) {