⬢ DragonFlyBSD Kernel Audit
DF-5320 / fix.diff
← back to finding ↓ download raw
# DF-5320 — unified diff (authored post-verification, NOT applied to sys/)

Root causes addressed in this file:
1. struct tty outlives the softc forever, but ucom_detach_tty() leaves
   t_sc/t_oproc/t_param/t_stop pointing into the softc that
   device_set_driver() kfrees when the driver's detach returns.
   => clear the driver callbacks while still holding tp->t_token and the
      sc lock (t_stop must become nottystop: ttyflush() calls t_stop
      unconditionally, tty.c:1441).
2. ucom_dev_read/ucom_dev_write deref tp->t_sc with only a KKASSERT;
   after (1) t_sc can legitimately be NULL for a detached-but-open tty.
   => return EIO instead.

This kills every tty-layer entry into the freed softc (ttyflush -> t_stop,
ttioctl/TIOCSETA -> t_param, ttstart -> t_oproc) which is the *permanent*
dangling-reference sink.  The residual in-flight cdev-dispatch window
(si_drv1 used by a syscall that already passed devfs_fo_ioctl's v_rdev
check when the kfree lands) is a devfs/core problem — nothing in this file
can close it; a full fix needs either a real dead_dev_ops revector in
destroy_dev (as documented but unimplemented in kern_conf.c) or a
completion barrier before device_set_driver() frees the softc.

--- a/sys/bus/u4b/serial/usb_serial.c
+++ b/sys/bus/u4b/serial/usb_serial.c
@@ -582,6 +582,20 @@ ucom_detach_tty(struct ucom_super_softc *ssc, struct ucom_softc *sc)
 		if (sc->sc_callback->ucom_stop_write) {
 			(sc->sc_callback->ucom_stop_write) (sc);
 		}
+
+		/*
+		 * The driver softc embedding this ucom_softc is freed by
+		 * device_set_driver() as soon as the driver's detach
+		 * routine returns, but this struct tty is never freed.
+		 * Detach the driver callbacks and clear t_sc so that no
+		 * later tty-layer entry (ttyflush() -> t_stop, ttioctl()
+		 * -> t_param, ttstart() -> t_oproc) can dereference the
+		 * freed softc.  t_stop must be nottystop() because
+		 * ttyflush() invokes it unconditionally.
+		 */
+		tp->t_oproc = NULL;
+		tp->t_param = NULL;
+		tp->t_stop = nottystop;
+		tp->t_sc = NULL;
 		UCOM_MTX_UNLOCK(sc);
 		lwkt_reltoken(&tp->t_token);
 	} else {
@@ -1038,7 +1052,10 @@ ucom_dev_read(struct dev_read_args *ap)
 	tp = dev->si_tty;
 	lwkt_gettoken(&tp->t_token);
 	KKASSERT(tp!=NULL);
-	sc = tp->t_sc;
-	KKASSERT(sc!=NULL);
+	sc = tp->t_sc;
+	if (sc == NULL) {		/* tty outlives a detached device */
+		lwkt_reltoken(&tp->t_token);
+		return (EIO);
+	}
 
 	DPRINTF("tp = %p, flag = 0x%x\n", tp, ap->a_ioflag);
@@ -1065,7 +1082,10 @@ ucom_dev_write(struct dev_write_args *ap)
 	tp = dev->si_tty;
 	lwkt_gettoken(&tp->t_token);
 	KKASSERT(tp!=NULL);
-	sc = tp->t_sc;
-	KKASSERT(sc!=NULL);
+	sc = tp->t_sc;
+	if (sc == NULL) {		/* tty outlives a detached device */
+		lwkt_reltoken(&tp->t_token);
+		return (EIO);
+	}
 
 	DPRINTF("tp = %p, flag = 0x%x\n", tp, ap->a_ioflag);