DF-5212 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 | # 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)) { |