⬢ DragonFlyBSD Kernel Audit
DF-4762 / fix.diff
← back to finding ↓ download raw 
  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
# DF-4762 fix — serialize RX chunk processing against fwe_stop() teardown

NOT applied to sys/ (read-only audit tree). Authored after source-level
verification; not runtime-validated (no FireWire hardware on the audit
guest) — see VERDICT.md.

Design notes:
 * DragonFly lwkt serializers are NOT recursive (sys/sys/serialize.h:42),
   so fwohci_rbuf_update() must snapshot `hand` under the serializer and
   invoke it after releasing it; fwe_as_input() revalidates the channel
   after it acquires the serializer itself.
 * fwe_stop() must close the fwohci_intr() guard windows
   (FWXFERQ_OPEN at fwohci.c:1813, FWXFERQ_HANDLER at fwohci.c:2183)
   BEFORE the 1-second tsleep inside fwohci_irx_disable(), and all
   chunk-queue mutation in fwohci_rbuf_update() must happen under the
   xferq serializer so it cannot interleave with the kfree().

--- a/sys/bus/firewire/firewirereg.h
+++ b/sys/bus/firewire/firewirereg.h
@@ -200,5 +200,6 @@ struct fw_xferq {
 	struct kqinfo rkq;
 	caddr_t sc;
 	void (*hand) (struct fw_xferq *);
+	struct lwkt_serializer *slz;	/* optional: guards chunk qs + hand */
 };
 
 struct fw_bulkxfer{
--- a/sys/bus/firewire/fwohci.c
+++ b/sys/bus/firewire/fwohci.c
@@ -2143,6 +2143,8 @@ fwohci_rbuf_update(struct fwohci_softc *sc, int dmach)
 	ir = fc->ir[dmach];
 	ldesc = sc->ir[dmach].ndesc - 1;
 #if 0
 	dump_db(sc, dmach);
 #endif
+	if (ir->slz != NULL)
+		lwkt_serialize_enter(ir->slz);
 	crit_enter();
 	fwdma_sync_multiseg_all(sc->ir[dmach].am, BUS_DMASYNC_POSTREAD);
 	while ((chunk = STAILQ_FIRST(&ir->stdma)) != NULL) {
@@ -2181,9 +2183,15 @@ fwohci_rbuf_update(struct fwohci_softc *sc, int dmach)
 		w++;
 	}
 	crit_exit();
-	if (w) {
-		if (ir->flag & FWXFERQ_HANDLER)
-			ir->hand(ir);
+	hand = (w ? ir->hand : NULL);	/* snapshot under slz */
+	if (ir->slz != NULL)
+		lwkt_serialize_exit(ir->slz);
+	if (w) {
+		if (hand != NULL)
+			hand(ir);	/* handler re-checks under its own serializer */
 		else
 			wakeup(ir);
 	}
 }
--- a/sys/dev/netif/fwe/if_fwe.c
+++ b/sys/dev/netif/fwe/if_fwe.c
@@ -261,11 +261,24 @@ fwe_stop(struct fwe_softc *fwe)
 
 	if (fwe->dma_ch >= 0) {
 		xferq = fc->ir[fwe->dma_ch];
 
+		/*
+		 * Stop accepting new RX completions BEFORE disabling the
+		 * DMA: fwohci_intr()/fwohci_rbuf_update() gate on
+		 * FWXFERQ_OPEN and FWXFERQ_HANDLER, and fwohci_irx_disable()
+		 * sleeps for up to 1s with those flags still set otherwise.
+		 * All chunk-queue mutation in fwohci_rbuf_update() happens
+		 * under xferq->slz (== if_serializer), so after we hold the
+		 * serializer no completion can reference bulkxfer anymore.
+		 */
+		xferq->flag &= ~(FWXFERQ_OPEN | FWXFERQ_HANDLER |
+				FWXFERQ_STREAM | FWXFERQ_CHTAGMASK);
+		xferq->hand = NULL;
+
 		if (xferq->flag & FWXFERQ_RUNNING)
 			fc->irx_disable(fc, fwe->dma_ch);
 		xferq->flag &= 
 			~(FWXFERQ_MODEMASK | FWXFERQ_OPEN | FWXFERQ_STREAM |
 			FWXFERQ_EXTBUF | FWXFERQ_HANDLER | FWXFERQ_CHTAGMASK);
 		xferq->hand =  NULL;
+		xferq->slz = NULL;
@@ -331,6 +344,7 @@ fwe_init(void *arg)
 		/* register fwe_input handler */
 		xferq->sc = (caddr_t) fwe;
 		xferq->hand = fwe_as_input;
+		xferq->slz = ifp->if_serializer;
 		xferq->bnchunk = rx_queue_len;
@@ -541,6 +555,13 @@ fwe_as_input(struct fw_xferq *xferq)
 	fwe = (struct fwe_softc *)xferq->sc;
 	ifp = &fwe->fwe_if;
 	lwkt_serialize_enter(ifp->if_serializer);
+	if (xferq->hand != fwe_as_input) {
+		/* channel was torn down while we waited for the serializer */
+		lwkt_serialize_exit(ifp->if_serializer);
+		return;
+	}
 	while ((sxfer = STAILQ_FIRST(&xferq->stvalid)) != NULL) {