git: c84c2bf78b39 - main - ufshci: fix WLUN periph reference counting
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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;
}