⬢ DragonFlyBSD Kernel Audit
DF-4097 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/disk/buslogic/bt.c
+++ b/sys/dev/disk/buslogic/bt.c
@@ -887,12 +887,14 @@
 		KKASSERT(lockowned(&bt->lock));
 	if ((bccb = SLIST_FIRST(&bt->free_bt_ccbs)) != NULL) {
 		SLIST_REMOVE_HEAD(&bt->free_bt_ccbs, links);
+		bccb->gen++;		/* DF-4101 generation handshake */
 		bt->active_ccbs++;
 	} else {
 		btallocccbs(bt);
 		bccb = SLIST_FIRST(&bt->free_bt_ccbs);
 		if (bccb != NULL) {
 			SLIST_REMOVE_HEAD(&bt->free_bt_ccbs, links);
+			bccb->gen++;	/* DF-4101 generation handshake */
 			bt->active_ccbs++;
 		}
 	}
@@ -1154,6 +1156,19 @@
 		u_int32_t secs_per_cylinder;
 
 		ccg = &ccb->ccg;
+		/*
+		 * DF-2429: block_size is device-fed (READ CAPACITY, da(4)
+		 * passes it through unvalidated).  0 or > 1MiB makes the
+		 * size_mb computation below divide by zero and panics the
+		 * kernel; the XPT layer only filters block_size == 0 and
+		 * volume_size == 0, not the > 1MiB case that zeroes the
+		 * inner quotient.
+		 */
+		if (ccg->block_size == 0 || ccg->block_size > (1024L * 1024L)) {
+			ccb->ccb_h.status = CAM_REQ_INVALID;
+			xpt_done(ccb);
+			break;
+		}
 		size_mb = ccg->volume_size
 			/ ((1024L * 1024L) / ccg->block_size);
 		
@@ -1282,7 +1297,14 @@
 		bus_dmamap_sync(bt->buffer_dmat, bccb->dmamap, op);
 
 	} else {
-		bccb->hccb.opcode = INITIATOR_CCB;
+		/*
+		 * DF-4097: XPT_RESET_DEV already stored
+		 * INITIATOR_BUS_DEV_RESET in btaction(); do not clobber a
+		 * device reset into an ordinary opcode-0 CCB with
+		 * cmd_len == 0 that conforming firmware must reject.
+		 */
+		if (bccb->hccb.opcode != INITIATOR_BUS_DEV_RESET)
+			bccb->hccb.opcode = INITIATOR_CCB;
 		bccb->hccb.data_len = 0;
 		bccb->hccb.data_addr = 0;
 	}
@@ -1303,6 +1325,7 @@
 	ccb->ccb_h.status |= CAM_SIM_QUEUED;
 	LIST_INSERT_HEAD(&bt->pending_ccbs, &ccb->ccb_h, sim_links.le);
 
+	bccb->timer_gen = bccb->gen;	/* DF-4101 */
 	callout_reset(&bccb->timer, (ccb->ccb_h.timeout * hz) / 1000,
 	    bttimeout, bccb);
 
@@ -1443,9 +1466,20 @@
 			}
 		}
 		device_printf(bt->dev, "No longer in timeout\n");
+		/* DF-4100: release the recovery BCCB for the next BDR */
+		bt->recovery_bccb->flags = BCCB_FREE;
 		return;
 	}
 
+	/* DF-4101: callout_stop() does not interlock against a bttimeout()
+	 * that has already fired and is blocked acquiring bt->lock.  Use
+	 * the generation handshake so a stale handler that runs after the
+	 * CCB was completed and recycled cannot act on the new command. */
+	if (bccb->gen != bccb->timer_gen) {
+		/* stale bttimeout on a recycled CCB: nothing to do */
+		callout_stop(&bccb->timer);
+		return;
+	}
 	callout_stop(&bccb->timer);
 
 	switch (comp_code) {
@@ -1514,17 +1548,29 @@
 			csio->ccb_h.status = CAM_SEQUENCE_FAIL;
 			break;
 		case BTSTAT_INVALID_ACTION_CODE:
-			panic("%s: Invalid Action code", bt_name(bt));
+			/*
+			 * DF-4098: btstat is a host-adapter-fed byte;
+			 * never trust it with the kernel's life.
+			 */
+			device_printf(bt->dev,
+			    "btdone - Invalid Action code from adapter\n");
+			csio->ccb_h.status = CAM_REQ_CMP_ERR;
 			break;
 		case BTSTAT_INVALID_OPCODE:
-			panic("%s: Invalid CCB Opcode code", bt_name(bt));
+			device_printf(bt->dev,
+			    "btdone - Invalid CCB Opcode code from adapter\n");
+			csio->ccb_h.status = CAM_REQ_CMP_ERR;
 			break;
 		case BTSTAT_LINKED_CCB_LUN_MISMATCH:
 			/* We don't even support linked commands... */
-			panic("%s: Linked CCB Lun Mismatch", bt_name(bt));
+			device_printf(bt->dev,
+			    "btdone - Linked CCB Lun Mismatch\n");
+			csio->ccb_h.status = CAM_REQ_CMP_ERR;
 			break;
 		case BTSTAT_INVALID_CCB_OR_SG_PARAM:
-			panic("%s: Invalid CCB or SG list", bt_name(bt));
+			device_printf(bt->dev,
+			    "btdone - Invalid CCB or SG list\n");
+			csio->ccb_h.status = CAM_REQ_CMP_ERR;
 			break;
 		case BTSTAT_AUTOSENSE_FAILED:
 			csio->ccb_h.status = CAM_AUTOSENSE_FAIL;
@@ -1605,6 +1651,20 @@
 		btfreeccb(bt, bccb);
 		xpt_done(ccb);
 		break;
+	default:
+		/*
+		 * DF-4099: comp_code is a host-adapter-fed byte covering
+		 * 256 values; anything unhandled must still complete (and
+		 * free) the CCB, otherwise the command hangs forever with
+		 * its timeout already stopped and the BCCB is lost.
+		 */
+		device_printf(bt->dev,
+		    "btdone - unknown mailbox completion code 0x%x\n",
+		    comp_code);
+		csio->ccb_h.status = CAM_REQ_CMP_ERR;
+		btfreeccb(bt, bccb);
+		xpt_done(ccb);
+		break;
 	}
 }
 
@@ -1647,8 +1707,10 @@
 		DELAY(100);
 	}
 	if (timeout == 0) {
-		panic("%s: btreset - Diagnostic Active failed to drop. "
-		       "status = 0x%x\n", bt_name(bt), status);
+		/* DF-4098: hostile or wedged hardware must not panic */
+		device_printf(bt->dev,
+		    "btreset - Diagnostic Active failed to drop. "
+		    "status = 0x%x\n", status);
 		return (ETIMEDOUT);
 	}
 
@@ -2151,6 +2213,17 @@
 		lockmgr(&bt->lock, LK_RELEASE);
 		return;
 	}
+	if (bccb->gen != bccb->timer_gen) {
+		/* DF-4101: this bttimeout fired for a previous command that
+		 * used this BCCB; the BCCB has since been recycled for a
+		 * new, healthy command.  Do not touch it. */
+		xpt_print_path(ccb->ccb_h.path);
+		kprintf("CCB %p - stale timeout (gen %u != timer_gen %u) "
+		       "ignored\n", (void *)bccb, bccb->gen,
+		       bccb->timer_gen);
+		lockmgr(&bt->lock, LK_RELEASE);
+		return;
+	}
 
 	/*
 	 * In order to simplify the recovery process, we ask the XPT
@@ -2214,6 +2287,12 @@
 		bccb->flags |= BCCB_DEVICE_RESET;
 		callout_reset(&bccb->timer, 2 * hz, bttimeout, bccb);
 
+		/*
+		 * DF-4100: mark the recovery BCCB active so that its
+		 * completion passes the btdone() BCCB_ACTIVE guard and
+		 * actually runs the recovery branch.
+		 */
+		bt->recovery_bccb->flags = BCCB_ACTIVE;
 		bt->recovery_bccb->hccb.opcode = INITIATOR_BUS_DEV_RESET;
 
 		/* No Data Transfer */
--- a/sys/dev/disk/buslogic/btreg.h
+++ b/sys/dev/disk/buslogic/btreg.h
@@ -556,6 +556,8 @@
 	struct	bt_hccb		 hccb;
 	SLIST_ENTRY(bt_ccb)	 links;
 	u_int32_t		 flags;
+	u_int32_t		 gen;		/* DF-4101: reuse generation */
+	u_int32_t		 timer_gen;	/* DF-4101: gen timer fired for */
 	union ccb		*ccb;
 	bus_dmamap_t		 dmamap;
 	struct callout		 timer;