⬢ DragonFlyBSD Kernel Audit
DF-5328 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/cam/cam_periph.c
+++ b/sys/bus/cam/cam_periph.c
@@ -169,15 +169,45 @@ cam_periph_alloc(periph_ctor_t *periph_ctor,
 	init_level = 0;
 	/*
 	 * Handle Hot-Plug scenarios.  If there is already a peripheral
 	 * of our type assigned to this path, we are likely waiting for
 	 * final close on an old, invalidated, peripheral.  If this is
 	 * the case, queue up a deferred call to the peripheral's async
 	 * handler.  If it looks like a mistaken re-allocation, complain.
+	 *
+	 * DF-5328: cam_periph_find() returns a borrowed pointer after
+	 * dropping the buses lock; touching periph->flags / deferred_*
+	 * unlocked races camperiphfree() (buses-lock-held TAILQ_REMOVE,
+	 * dtor, kfree).  Perform the lookup AND the deferred bookkeeping
+	 * under one buses-lock hold so it serializes with camperiphfree():
+	 * either the periph was already removed from the units list (we
+	 * see NULL), or our stores complete before camperiphfree() reads
+	 * the flag at the top of its deferred-callback block.  Store
+	 * deferred_callback/deferred_ac BEFORE the NEW_DEV_FOUND flag so
+	 * camperiphfree() can never observe the flag with an unpublished
+	 * (NULL) callback pointer.
 	 */
-	if ((periph = cam_periph_find(path, name)) != NULL) {
+	xpt_lock_buses();
+	periph = NULL;
+	for (p_drv = periph_drivers; *p_drv != NULL; p_drv++) {
+		if (strcmp((*p_drv)->driver_name, name) == 0)
+			break;
+	}
+	if (*p_drv != NULL) {
+		TAILQ_FOREACH(periph, &(*p_drv)->units, unit_links) {
+			if (xpt_path_comp(periph->path, path) == 0)
+				break;
+		}
+	}
+	if (periph != NULL) {
 
 		if ((periph->flags & CAM_PERIPH_INVALID) != 0
 		 && (periph->flags & CAM_PERIPH_NEW_DEV_FOUND) == 0) {
-			periph->flags |= CAM_PERIPH_NEW_DEV_FOUND;
 			periph->deferred_callback = ac_callback;
 			periph->deferred_ac = code;
+			cpu_sfence();
+			periph->flags |= CAM_PERIPH_NEW_DEV_FOUND;
+			xpt_unlock_buses();
 			return (CAM_REQ_INPROG);
 		} else {
+			xpt_unlock_buses();
 			kprintf("cam_periph_alloc: attempt to re-allocate "
 			       "valid device %s%d rejected\n",
 			       periph->periph_name, periph->unit_number);
+			return (CAM_REQ_INVALID);
 		}
-		return (CAM_REQ_INVALID);
 	}
+	xpt_unlock_buses();
--- a/sys/bus/cam/cam_periph.c
+++ b/sys/bus/cam/ciphelper_note
@@ -299,6 +299,11 @@ cam_periph_find(struct cam_path *path, char *name)
 	xpt_unlock_buses();
 	return(NULL);
 }
+
+/*
+ * NB: cam_periph_find() drops the buses lock before returning.  The
+ * returned periph is borrowed: a caller that keeps using it must hold
+ * its own reference or perform the lookup under the buses lock (see
+ * cam_periph_alloc()).
+ */