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

Jaeyoon Choi <[email protected]>
Newsgroups gmane.os.freebsd.devel.cvs.src
Message-ID <6a7937f9.1d03b.57d7ace6__48764.470806212$1786329158$gmane$org@gitrepo.freebsd.org>
The branch main has been updated by jaeyoon:

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

commit c84c2bf78b39058a339551117323668c933f6162
Author:     Jaeyoon Choi <[email protected]>
AuthorDate: 2026-08-10 01:39:12 +0000
Commit:     Jaeyoon Choi <[email protected]>
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;
 	}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.