git: c84c2bf78b39 - main - ufshci: fix WLUN periph reference counting

From: Jaeyoon Choi <jaeyoon_at_FreeBSD.org>
Date: Mon, 10 Aug 2026 02:31:21 UTC
The branch main has been updated by jaeyoon:

URL: https://cgit.FreeBSD.org/src/commit/?id=c84c2bf78b39058a339551117323668c933f6162

commit c84c2bf78b39058a339551117323668c933f6162
Author:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
AuthorDate: 2026-08-10 01:39:12 +0000
Commit:     Jaeyoon Choi <jaeyoon@FreeBSD.org>
CommitDate: 2026-08-10 02:28:49 +0000

    ufshci: fix WLUN periph reference counting
    
    The driver stored the WLUN periph pointer without holding a reference,
    so the pointer went stale when the pass(4) device went away. In
    addition, ufshci_sim_send_ssu() released a reference that it had never
    acquired.
    
    Define a simple ownership rule. ufshci_sim_find_periph() acquires the
    periph and returns it. The cache owns one reference. The controller
    destructor drops it with cam_periph_release() before taking the SIM
    lock, since the release takes the CAM device lock by itself.
    ufshci_sim_send_ssu() acquires its own reference and releases it when
    done. Reuse the cached periph instead of searching again, so the old
    reference is not leaked.
    
    Sponsored by:           Samsung Electronics
    Reviewed by:            imp (mentor)
    Differential Revision:  https://reviews.freebsd.org/D58658
---
 sys/dev/ufshci/ufshci_ctrlr.c   |  2 ++
 sys/dev/ufshci/ufshci_dev.c     | 13 ++++++++-----
 sys/dev/ufshci/ufshci_private.h |  1 +
 sys/dev/ufshci/ufshci_sim.c     | 36 +++++++++++++++++++++++++++++++-----
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/sys/dev/ufshci/ufshci_ctrlr.c b/sys/dev/ufshci/ufshci_ctrlr.c
index cb5549f8e7c9..421aa1e28124 100644
--- a/sys/dev/ufshci/ufshci_ctrlr.c
+++ b/sys/dev/ufshci/ufshci_ctrlr.c
@@ -448,6 +448,8 @@ ufshci_ctrlr_destruct(struct ufshci_controller *ctrlr, device_t dev)
 		bus_release_resource(ctrlr->dev, SYS_RES_IRQ,
 		    rman_get_rid(ctrlr->res), ctrlr->res);
 
+	ufshci_sim_release_wlun_periph(ctrlr);
+
 	mtx_lock(&ctrlr->sc_mtx);
 
 	ufshci_sim_detach(ctrlr);
diff --git a/sys/dev/ufshci/ufshci_dev.c b/sys/dev/ufshci/ufshci_dev.c
index c53d9b71ba2c..7113834db13a 100644
--- a/sys/dev/ufshci/ufshci_dev.c
+++ b/sys/dev/ufshci/ufshci_dev.c
@@ -508,12 +508,15 @@ ufshci_dev_init_ufs_power_mode(struct ufshci_controller *ctrlr)
 	if (ctrlr->quirks & UFSHCI_QUIRK_SKIP_WELL_KNOWN_LUNS)
 		return (0);
 
-	ctrlr->ufs_device_wlun_periph = ufshci_sim_find_periph(ctrlr,
-	    UFSHCI_WLUN_UFS_DEVICE);
 	if (ctrlr->ufs_device_wlun_periph == NULL) {
-		ufshci_printf(ctrlr,
-		    "Well-known LUN `UFS Device (0x50)` not found\n");
-		return (0);
+		/* The returned reference is kept by the cached pointer. */
+		ctrlr->ufs_device_wlun_periph = ufshci_sim_find_periph(ctrlr,
+		    UFSHCI_WLUN_UFS_DEVICE);
+		if (ctrlr->ufs_device_wlun_periph == NULL) {
+			ufshci_printf(ctrlr,
+			    "Well-known LUN `UFS Device (0x50)` not found\n");
+			return (0);
+		}
 	}
 
 	ctrlr->ufs_dev.power_mode_supported = true;
diff --git a/sys/dev/ufshci/ufshci_private.h b/sys/dev/ufshci/ufshci_private.h
index d4c4140f82ae..d87651a46a2a 100644
--- a/sys/dev/ufshci/ufshci_private.h
+++ b/sys/dev/ufshci/ufshci_private.h
@@ -434,6 +434,7 @@ uint8_t ufshci_sim_translate_scsi_to_ufs_lun(lun_id_t scsi_lun);
 uint64_t ufshci_sim_translate_ufs_to_scsi_lun(uint8_t ufs_lun);
 int ufshci_sim_attach(struct ufshci_controller *ctrlr);
 void ufshci_sim_detach(struct ufshci_controller *ctrlr);
+void ufshci_sim_release_wlun_periph(struct ufshci_controller *ctrlr);
 struct cam_periph *ufshci_sim_find_periph(struct ufshci_controller *ctrlr,
     uint8_t wlun);
 int ufshci_sim_send_ssu(struct ufshci_controller *ctrlr, bool start,
diff --git a/sys/dev/ufshci/ufshci_sim.c b/sys/dev/ufshci/ufshci_sim.c
index 148840123d13..9d80dd195ad9 100644
--- a/sys/dev/ufshci/ufshci_sim.c
+++ b/sys/dev/ufshci/ufshci_sim.c
@@ -393,6 +393,20 @@ ufshci_sim_attach(struct ufshci_controller *ctrlr)
 	return (0);
 }
 
+/*
+ * Drop the cached WLUN periph reference. cam_periph_release() takes the
+ * CAM device lock itself, so call this without sc_mtx held: CAM takes
+ * the device lock before the SIM lock, not the other way around.
+ */
+void
+ufshci_sim_release_wlun_periph(struct ufshci_controller *ctrlr)
+{
+	if (ctrlr->ufs_device_wlun_periph != NULL) {
+		cam_periph_release(ctrlr->ufs_device_wlun_periph);
+		ctrlr->ufs_device_wlun_periph = NULL;
+	}
+}
+
 void
 ufshci_sim_detach(struct ufshci_controller *ctrlr)
 {
@@ -425,6 +439,10 @@ ufshci_sim_detach(struct ufshci_controller *ctrlr)
 	}
 }
 
+/*
+ * On success this returns a referenced periph; the caller is responsible
+ * for dropping the reference with cam_periph_release().
+ */
 struct cam_periph *
 ufshci_sim_find_periph(struct ufshci_controller *ctrlr, uint8_t wlun)
 {
@@ -446,6 +464,8 @@ ufshci_sim_find_periph(struct ufshci_controller *ctrlr, uint8_t wlun)
 	while (1) {
 		xpt_path_lock(path);
 		periph = cam_periph_find(path, "pass");
+		if (periph != NULL && cam_periph_acquire(periph) != 0)
+			periph = NULL;
 		xpt_path_unlock(path);
 
 		if (periph)
@@ -474,17 +494,23 @@ ufshci_sim_send_ssu(struct ufshci_controller *ctrlr, bool start,
 	union ccb *ccb;
 	int err;
 
-	/* Acquire periph reference */
-	if (periph && cam_periph_acquire(periph) != 0) {
+	/* Acquire a periph reference for the duration of this call. */
+	if (periph != NULL && cam_periph_acquire(periph) != 0) {
+		/* The cached periph is going away; drop its reference. */
+		cam_periph_release(periph);
+		ctrlr->ufs_device_wlun_periph = NULL;
 		periph = NULL;
 	}
 
 	if (periph == NULL) {
-		/* If the periph device does not exist, it will try to find it
-		 * again */
+		/*
+		 * If the periph device does not exist, try to find it again.
+		 * The reference returned by ufshci_sim_find_periph() is used
+		 * for this call; take an extra one for the cached pointer.
+		 */
 		periph = ufshci_sim_find_periph(ctrlr,
 		    (uint8_t)UFSHCI_WLUN_UFS_DEVICE);
-		if (periph)
+		if (periph != NULL && cam_periph_acquire(periph) == 0)
 			ctrlr->ufs_device_wlun_periph = periph;
 	}