[PATCH v2 0/2] cxl: dport fixes from sashiko reports

[PATCH v2 0/2] cxl: dport fixes from sashiko reports

[Dave Jiang]

Couple dport fixes resulted from sashiko pre-existing issue reports.
There are no depedencies between the two patches.

v2:
- Addressed couple things from Jonathan. See patch for revlog.

Dave Jiang (2):
  cxl/port: Clear cached dport pointers when a dport is removed
  cxl/core: Hold the dport host lock across dport lookup and use

 drivers/cxl/core/core.h    |  6 +---
 drivers/cxl/core/pci.c     |  8 ++++-
 drivers/cxl/core/port.c    | 65 ++++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/ras_rch.c | 12 ++++++-
 drivers/cxl/cxl.h          | 16 ++++++++++
 drivers/cxl/mem.c          | 14 +++++---
 drivers/cxl/pci.c          | 11 ++++---
 7 files changed, 115 insertions(+), 17 deletions(-)


base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.54.0

[PATCH v2 1/2] cxl/port: Clear cached dport pointers when a dport is removed

[Dave Jiang]

Switch decoders cache dport pointers in cxlsd->target[], and nothing
clears them when a dport is freed. Readers then dereference freed memory.

KASAN caught it under a cxl_test load/unload loop with concurrent sysfs
reads (abbreviated).

Clear the matching slots from cxl_dport_remove(), walking the port's
decoders the way cxl_port_update_decoder_targets() does on the add side.
Scan all nr_targets slots, the target[] allocation size, and clear every
hit.

Fixes: 8330671c57c7 ("cxl: Add helper to delete dport")
Cc: stable@vger.kernel.org
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Assisted-by: LLM
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
---
Found by KASAN while stress-testing a separate dport locking fix, not from a
failure report.

Reproducer: cxl_test, CONFIG_KASAN=y, 12 concurrent readers cat'ing
/sys/bus/cxl/devices/*/target_list while cxl_test is unloaded and reloaded.

With the fix reverted the UAF lands ~1s into the first unload, on the trace
above. With it applied, 36 cycles across two runs are clean: no KASAN
report, no WARNING, lockdep never self-disables. clear_decoder_target()
fired 2916 times during those runs, so the new path ran under the load.

KASAN only reports once per boot unless kasan_multi_shot is set
(report_enabled(), mm/kasan/report.c), so the reverted-fix run cannot show
a per-cycle count - one report is the ceiling, not the frequency.

target_list output is unaffected. Clearing a slot truncates the list at the
first NULL, so that was the regression to watch for: 88 target_list files,
no empty values before or after, and the multiset of values is unchanged
once the ida-assigned decoder names are stripped.

Enumeration is unchanged too: 15 memdev, 12 port, 15 endpoint, 192 decoder.
---
 drivers/cxl/core/port.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 625e4aa427db..6ff3353865e3 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1089,11 +1089,40 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
 		device_unlock(&port->dev);
 }
 
+static int clear_decoder_target(struct device *dev, void *data)
+{
+	struct cxl_dport *dport = data;
+	struct cxl_switch_decoder *cxlsd;
+
+	if (!is_switch_decoder(dev))
+		return 0;
+
+	cxlsd = to_cxl_switch_decoder(dev);
+	guard(rwsem_write)(&cxl_rwsem.region);
+
+	/*
+	 * A dport can occupy more than one position of an interleave, so
+	 * scan the whole target list rather than stopping at the first hit.
+	 */
+	for (int i = 0; i < cxlsd->nr_targets; i++) {
+		if (cxlsd->target[i] != dport)
+			continue;
+		cxlsd->target[i] = NULL;
+		dev_dbg(dev, "dport%d removed from target list, index %d\n",
+			dport->port_id, i);
+	}
+
+	return 0;
+}
+
 static void cxl_dport_remove(void *data)
 {
 	struct cxl_dport *dport = data;
 	struct cxl_port *port = dport->port;
 
+	/* counterpart of cxl_port_update_decoder_targets() */
+	device_for_each_child(&port->dev, dport, clear_decoder_target);
+
 	port->nr_dports--;
 	xa_erase(&port->dports, (unsigned long) dport->dport_dev);
 	put_device(dport->dport_dev);
-- 
2.54.0

Re: [PATCH v2 1/2] cxl/port: Clear cached dport pointers when a dport is removed

[Li Ming]

在 2026/9/29 07:03, Dave Jiang 写道:
> Switch decoders cache dport pointers in cxlsd->target[], and nothing
> clears them when a dport is freed. Readers then dereference freed memory.
>
> KASAN caught it under a cxl_test load/unload loop with concurrent sysfs
> reads (abbreviated).
>
> Clear the matching slots from cxl_dport_remove(), walking the port's
> decoders the way cxl_port_update_decoder_targets() does on the add side.
> Scan all nr_targets slots, the target[] allocation size, and clear every
> hit.
>
> Fixes: 8330671c57c7 ("cxl: Add helper to delete dport")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Assisted-by: LLM
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Li Ming <ming.li@zohomail.com>

> ---
> Found by KASAN while stress-testing a separate dport locking fix, not from a
> failure report.
>
> Reproducer: cxl_test, CONFIG_KASAN=y, 12 concurrent readers cat'ing
> /sys/bus/cxl/devices/*/target_list while cxl_test is unloaded and reloaded.
>
> With the fix reverted the UAF lands ~1s into the first unload, on the trace
> above. With it applied, 36 cycles across two runs are clean: no KASAN
> report, no WARNING, lockdep never self-disables. clear_decoder_target()
> fired 2916 times during those runs, so the new path ran under the load.
>
> KASAN only reports once per boot unless kasan_multi_shot is set
> (report_enabled(), mm/kasan/report.c), so the reverted-fix run cannot show
> a per-cycle count - one report is the ceiling, not the frequency.
>
> target_list output is unaffected. Clearing a slot truncates the list at the
> first NULL, so that was the regression to watch for: 88 target_list files,
> no empty values before or after, and the multiset of values is unchanged
> once the ida-assigned decoder names are stripped.
>
> Enumeration is unchanged too: 15 memdev, 12 port, 15 endpoint, 192 decoder.
> ---
>   drivers/cxl/core/port.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 625e4aa427db..6ff3353865e3 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1089,11 +1089,40 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
>   		device_unlock(&port->dev);
>   }
>   
> +static int clear_decoder_target(struct device *dev, void *data)
> +{
> +	struct cxl_dport *dport = data;
> +	struct cxl_switch_decoder *cxlsd;
> +
> +	if (!is_switch_decoder(dev))
> +		return 0;
> +
> +	cxlsd = to_cxl_switch_decoder(dev);
> +	guard(rwsem_write)(&cxl_rwsem.region);
> +
> +	/*
> +	 * A dport can occupy more than one position of an interleave, so
> +	 * scan the whole target list rather than stopping at the first hit.
> +	 */
> +	for (int i = 0; i < cxlsd->nr_targets; i++) {
> +		if (cxlsd->target[i] != dport)
> +			continue;
> +		cxlsd->target[i] = NULL;
> +		dev_dbg(dev, "dport%d removed from target list, index %d\n",
> +			dport->port_id, i);
> +	}
> +
> +	return 0;
> +}
> +
>   static void cxl_dport_remove(void *data)
>   {
>   	struct cxl_dport *dport = data;
>   	struct cxl_port *port = dport->port;
>   
> +	/* counterpart of cxl_port_update_decoder_targets() */
> +	device_for_each_child(&port->dev, dport, clear_decoder_target);
> +
>   	port->nr_dports--;
>   	xa_erase(&port->dports, (unsigned long) dport->dport_dev);
>   	put_device(dport->dport_dev);

Re: [PATCH v2 1/2] cxl/port: Clear cached dport pointers when a dport is removed

[Gregory Price]

On Mon, Sep 28, 2026 at 04:03:40PM -0700, Dave Jiang wrote:
> Switch decoders cache dport pointers in cxlsd->target[], and nothing
> clears them when a dport is freed. Readers then dereference freed memory.
> 
> KASAN caught it under a cxl_test load/unload loop with concurrent sysfs
> reads (abbreviated).
> 
> Clear the matching slots from cxl_dport_remove(), walking the port's
> decoders the way cxl_port_update_decoder_targets() does on the add side.
> Scan all nr_targets slots, the target[] allocation size, and clear every
> hit.
> 
> Fixes: 8330671c57c7 ("cxl: Add helper to delete dport")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Assisted-by: LLM
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>

[PATCH v2 2/2] cxl/core: Hold the dport host lock across dport lookup and use

[Dave Jiang]

Callers of cxl_pci_find_port() and cxl_mem_find_port() can be handed a
'struct cxl_dport' that is already freed, and all four dereference it
without the lock that would prevent that.

A dport is kzalloc()'d into a devres group on its host device, so it dies
when that host unbinds. The host is &port->dev for a switch port and
port->uport_dev for the root port.

Both helpers return the dport through an output parameter. The caller's
port reference pins struct cxl_port, not the dport allocation, and the
lookup itself is a bare xa_load() under no lock.

Add cxl_port_dport_host() to name the device that pins a dport, and
cxl_pci_find_dport() / cxl_mem_find_dport() to look one up with that
device's lock held and asserted. Convert the four callers to use them and
bail if the dport is gone. cxl_port_add_dport() already follows this rule.

cxl_mem_probe() chose its endpoint devm host from dport->rch, which meant
dereferencing the dport before taking any lock. Use cxl_port_dport_host():
rch dports only ever land on the root port, so it selects the same device
without needing the dport.

Fixes: 733b57f262b0 ("cxl/pci: Early setup RCH dport component registers from RCRB")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Assisted-by: LLM
---
v2:
- Reword the cxl_port_dport_host() comment. (Jonathan)
- Drop the note on stale cxlsd->target[] pointers, fixed by patch 1. (Jonathan)

Found by review, not from a failure report.

Untested: two of the four converted sites add a new lock acquisition, and
neither executed under test.

  cxl_pci_setup_regs()        new guard() sits behind is_cxl_restricted(),
                              which needs a PCI_EXP_TYPE_RC_END device
  cxl_handle_rdport_errors()  needs an RCH dport AER error; cxl_test has no
                              injection path

Only cxl_pci_setup_regs() introduces a lock order lockdep has not already
seen: device_lock(&pdev->dev) from probe, then the dport host lock.

cxl_handle_rdport_errors() nests its new host lock inside the
device_lock(&cxlmd->dev) that core/ras.c:274 and :298 hold, but
cxl_mem_probe() takes that same pair in that same order and ran clean under
PROVE_LOCKING, so the edge itself is covered. What is untested there is the
path, not the ordering.

Reaching either needs a cxl-type3 attached directly to a host bridge so
pci_pcie_type() is PCI_EXP_TYPE_RC_END, which run_qemu.sh does not generate.

Tested in QEMU with cxl_test, PROVE_LOCKING and KASAN on, 8 module
load/unload cycles with concurrent sysfs readers. No lockdep splat, no
device_lock_assert() firing, enumeration unchanged (15 memdev, 12 port,
192 decoder before and after).

cxl_mem_find_dport() ran for 15 memdevs on each of 9 loads, covering both
cxl_port_dport_host() branches. The rch branch is covered too: "cxl_mem
mem2: endpoint9 added to root3" shows the new selection picking the same
parent dport->rch did.
---
 drivers/cxl/core/core.h    |  6 +-----
 drivers/cxl/core/pci.c     |  8 +++++++-
 drivers/cxl/core/port.c    | 36 ++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/ras_rch.c | 12 +++++++++++-
 drivers/cxl/cxl.h          | 16 ++++++++++++++++
 drivers/cxl/mem.c          | 14 +++++++++-----
 drivers/cxl/pci.c          | 11 ++++++-----
 7 files changed, 86 insertions(+), 17 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 35eaf636adc9..0a39e500029a 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -178,11 +178,7 @@ static inline struct device *port_to_host(struct cxl_port *port)
 
 static inline struct device *dport_to_host(struct cxl_dport *dport)
 {
-	struct cxl_port *port = dport->port;
-
-	if (is_cxl_root(port))
-		return port->uport_dev;
-	return &port->dev;
+	return cxl_port_dport_host(dport->port);
 }
 #ifdef CONFIG_CXL_RAS
 void cxl_ras_init(void);
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 9d807c1a002c..8a236e22bb3f 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -734,10 +734,16 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
 	if (rc && type == CXL_REGLOC_RBI_COMPONENT && is_cxl_restricted(pdev)) {
 		struct cxl_dport *dport;
 		struct cxl_port *port __free(put_cxl_port) =
-			cxl_pci_find_port(pdev, &dport);
+			cxl_pci_find_port(pdev, NULL);
 		if (!port)
 			return -EPROBE_DEFER;
 
+		guard(device)(cxl_port_dport_host(port));
+
+		dport = cxl_pci_find_dport(pdev, port);
+		if (!dport)
+			return -EPROBE_DEFER;
+
 		rc = cxl_rcrb_get_comp_regs(pdev, map, dport);
 		if (rc)
 			return rc;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 6ff3353865e3..336d7c0f5d98 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1428,6 +1428,10 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
  *
  * Return a 'struct cxl_port' with an elevated reference if found. Use
  * __free(put_cxl_port) to release.
+ *
+ * The port reference does not pin @dport, which is a devm allocation of
+ * cxl_port_dport_host(). Pass NULL and use cxl_pci_find_dport() or
+ * cxl_mem_find_dport() under that device's lock instead.
  */
 static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
 					       struct cxl_dport **dport)
@@ -1943,6 +1947,38 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");
 
+/**
+ * cxl_pci_find_dport - find the dport of @port that @pdev is below
+ * @pdev: PCI device below the dport
+ * @port: port to search, typically from cxl_pci_find_port()
+ *
+ * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
+ *	    must not use the result after dropping it.
+ */
+struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
+				     struct cxl_port *port)
+{
+	device_lock_assert(cxl_port_dport_host(port));
+	return cxl_find_dport_by_dev(port, pdev->dev.parent);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_pci_find_dport, "CXL");
+
+/**
+ * cxl_mem_find_dport - find the dport of @port that @cxlmd is below
+ * @cxlmd: memdev below the dport
+ * @port: port to search, typically from cxl_mem_find_port()
+ *
+ * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
+ *	    must not use the result after dropping it.
+ */
+struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
+				     struct cxl_port *port)
+{
+	device_lock_assert(cxl_port_dport_host(port));
+	return cxl_find_dport_by_dev(port, grandparent(&cxlmd->dev));
+}
+EXPORT_SYMBOL_NS_GPL(cxl_mem_find_dport, "CXL");
+
 static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
 				    struct cxl_port *port)
 {
diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
index e0e01aa5eba6..827a2f64d892 100644
--- a/drivers/cxl/core/ras_rch.c
+++ b/drivers/cxl/core/ras_rch.c
@@ -118,10 +118,20 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
 	int severity;
 
 	struct cxl_port *port __free(put_cxl_port) =
-		cxl_pci_find_port(pdev, &dport);
+		cxl_pci_find_port(pdev, NULL);
 	if (!port)
 		return;
 
+	/*
+	 * The dport may have been freed since the lookup above, so take the
+	 * lock that pins it and find it again.
+	 */
+	guard(device)(cxl_port_dport_host(port));
+
+	dport = cxl_pci_find_dport(pdev, port);
+	if (!dport)
+		return;
+
 	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
 		return;
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index cab8ce39f465..f08de094f412 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -701,6 +701,18 @@ static inline bool is_cxl_root(struct cxl_port *port)
 	return port->uport_dev == port->dev.parent;
 }
 
+/*
+ * Return the owner device of the devm allocations of @port's dports. They
+ * are freed when that device unbinds, so hold device_lock() on the returned
+ * device across any lookup and use of a 'struct cxl_dport'.
+ */
+static inline struct device *cxl_port_dport_host(struct cxl_port *port)
+{
+	if (is_cxl_root(port))
+		return port->uport_dev;
+	return &port->dev;
+}
+
 /* Address translation functions exported to cxl_translate test module only */
 int cxl_validate_translation_params(u8 eiw, u16 eig, int pos);
 u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
@@ -747,6 +759,10 @@ DEFINE_FREE(put_cxl_dax_region, struct cxl_dax_region *, if (!IS_ERR_OR_NULL(_T)
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
 void cxl_bus_rescan(void);
 void cxl_bus_drain(void);
+struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
+				     struct cxl_port *port);
+struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
+				     struct cxl_port *port);
 struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
 				   struct cxl_dport **dport);
 struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 798e5c369cfc..91137487ac1f 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -133,7 +133,7 @@ static int cxl_mem_probe(struct device *dev)
 		return rc;
 
 	struct cxl_port *parent_port __free(put_cxl_port) =
-		cxl_mem_find_port(cxlmd, &dport);
+		cxl_mem_find_port(cxlmd, NULL);
 	if (!parent_port) {
 		dev_err(dev, "CXL port topology not found\n");
 		return -ENXIO;
@@ -148,10 +148,7 @@ static int cxl_mem_probe(struct device *dev)
 		}
 	}
 
-	if (dport->rch)
-		endpoint_parent = parent_port->uport_dev;
-	else
-		endpoint_parent = &parent_port->dev;
+	endpoint_parent = cxl_port_dport_host(parent_port);
 
 	scoped_guard(device, endpoint_parent) {
 		if (!endpoint_parent->driver) {
@@ -160,6 +157,13 @@ static int cxl_mem_probe(struct device *dev)
 			return -ENXIO;
 		}
 
+		/* @dport is only valid while @endpoint_parent stays bound */
+		dport = cxl_mem_find_dport(cxlmd, parent_port);
+		if (!dport) {
+			dev_err(dev, "CXL topology dport not found\n");
+			return -ENXIO;
+		}
+
 		rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
 		if (rc)
 			return rc;
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index c7c91e8dc51d..6e1d7ee94c19 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -706,22 +706,23 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size
 	struct device *root_dev;
 	struct cxl_dport *dport;
 	struct cxl_port *root __free(put_cxl_port) =
-		cxl_mem_find_port(cxlmd, &dport);
+		cxl_mem_find_port(cxlmd, NULL);
 
 	if (!root)
 		return -ENXIO;
 
-	root_dev = root->uport_dev;
+	root_dev = cxl_port_dport_host(root);
 	if (!root_dev)
 		return -ENXIO;
 
-	if (!dport->regs.rcd_pcie_cap)
-		return -ENXIO;
-
 	guard(device)(root_dev);
 	if (!root_dev->driver)
 		return -ENXIO;
 
+	dport = cxl_mem_find_dport(cxlmd, root);
+	if (!dport || !dport->regs.rcd_pcie_cap)
+		return -ENXIO;
+
 	switch (width) {
 	case 2:
 		return sysfs_emit(buf, "%#x\n",
-- 
2.54.0

Re: [PATCH v2 2/2] cxl/core: Hold the dport host lock across dport lookup and use

[Li Ming]

在 2026/9/29 07:03, Dave Jiang 写道:
> Callers of cxl_pci_find_port() and cxl_mem_find_port() can be handed a
> 'struct cxl_dport' that is already freed, and all four dereference it
> without the lock that would prevent that.
>
> A dport is kzalloc()'d into a devres group on its host device, so it dies
> when that host unbinds. The host is &port->dev for a switch port and
> port->uport_dev for the root port.

In unbinding case, seems like CXL driver needs to check port's driver 
before accessing a dport of the port. I guess the dport in 
devm_cxl_enumerate_ports() and add_port_attach_ep() also needs to be 
protected by port's lock? Like cxl_add_ep() in 
devm_cxl_enumerate_ports(), it is invoked without port's lock, the dport 
could be freed during the function calling.

BTW, just wondering whether adding a dedicated "struct device" to dport 
makes more sense? use get_device()/put_device() to prevent UAF of dports.

>
> Both helpers return the dport through an output parameter. The caller's
> port reference pins struct cxl_port, not the dport allocation, and the
> lookup itself is a bare xa_load() under no lock.
>
> Add cxl_port_dport_host() to name the device that pins a dport, and
> cxl_pci_find_dport() / cxl_mem_find_dport() to look one up with that
> device's lock held and asserted. Convert the four callers to use them and
> bail if the dport is gone. cxl_port_add_dport() already follows this rule.
>
> cxl_mem_probe() chose its endpoint devm host from dport->rch, which meant
> dereferencing the dport before taking any lock. Use cxl_port_dport_host():
> rch dports only ever land on the root port, so it selects the same device
> without needing the dport.
>
> Fixes: 733b57f262b0 ("cxl/pci: Early setup RCH dport component registers from RCRB")
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Assisted-by: LLM
> ---
> v2:
> - Reword the cxl_port_dport_host() comment. (Jonathan)
> - Drop the note on stale cxlsd->target[] pointers, fixed by patch 1. (Jonathan)
>
> Found by review, not from a failure report.
>
> Untested: two of the four converted sites add a new lock acquisition, and
> neither executed under test.
>
>    cxl_pci_setup_regs()        new guard() sits behind is_cxl_restricted(),
>                                which needs a PCI_EXP_TYPE_RC_END device
>    cxl_handle_rdport_errors()  needs an RCH dport AER error; cxl_test has no
>                                injection path
>
> Only cxl_pci_setup_regs() introduces a lock order lockdep has not already
> seen: device_lock(&pdev->dev) from probe, then the dport host lock.
>
> cxl_handle_rdport_errors() nests its new host lock inside the
> device_lock(&cxlmd->dev) that core/ras.c:274 and :298 hold, but
> cxl_mem_probe() takes that same pair in that same order and ran clean under
> PROVE_LOCKING, so the edge itself is covered. What is untested there is the
> path, not the ordering.
>
> Reaching either needs a cxl-type3 attached directly to a host bridge so
> pci_pcie_type() is PCI_EXP_TYPE_RC_END, which run_qemu.sh does not generate.
>
> Tested in QEMU with cxl_test, PROVE_LOCKING and KASAN on, 8 module
> load/unload cycles with concurrent sysfs readers. No lockdep splat, no
> device_lock_assert() firing, enumeration unchanged (15 memdev, 12 port,
> 192 decoder before and after).
>
> cxl_mem_find_dport() ran for 15 memdevs on each of 9 loads, covering both
> cxl_port_dport_host() branches. The rch branch is covered too: "cxl_mem
> mem2: endpoint9 added to root3" shows the new selection picking the same
> parent dport->rch did.
> ---
>   drivers/cxl/core/core.h    |  6 +-----
>   drivers/cxl/core/pci.c     |  8 +++++++-
>   drivers/cxl/core/port.c    | 36 ++++++++++++++++++++++++++++++++++++
>   drivers/cxl/core/ras_rch.c | 12 +++++++++++-
>   drivers/cxl/cxl.h          | 16 ++++++++++++++++
>   drivers/cxl/mem.c          | 14 +++++++++-----
>   drivers/cxl/pci.c          | 11 ++++++-----
>   7 files changed, 86 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 35eaf636adc9..0a39e500029a 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -178,11 +178,7 @@ static inline struct device *port_to_host(struct cxl_port *port)
>   
>   static inline struct device *dport_to_host(struct cxl_dport *dport)
>   {
> -	struct cxl_port *port = dport->port;
> -
> -	if (is_cxl_root(port))
> -		return port->uport_dev;
> -	return &port->dev;
> +	return cxl_port_dport_host(dport->port);
>   }
>   #ifdef CONFIG_CXL_RAS
>   void cxl_ras_init(void);
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002c..8a236e22bb3f 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -734,10 +734,16 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>   	if (rc && type == CXL_REGLOC_RBI_COMPONENT && is_cxl_restricted(pdev)) {
>   		struct cxl_dport *dport;
>   		struct cxl_port *port __free(put_cxl_port) =
> -			cxl_pci_find_port(pdev, &dport);
> +			cxl_pci_find_port(pdev, NULL);
>   		if (!port)
>   			return -EPROBE_DEFER;
>   
> +		guard(device)(cxl_port_dport_host(port));
> +
> +		dport = cxl_pci_find_dport(pdev, port);
> +		if (!dport)
> +			return -EPROBE_DEFER;
> +
>   		rc = cxl_rcrb_get_comp_regs(pdev, map, dport);
>   		if (rc)
>   			return rc;
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 6ff3353865e3..336d7c0f5d98 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1428,6 +1428,10 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
>    *
>    * Return a 'struct cxl_port' with an elevated reference if found. Use
>    * __free(put_cxl_port) to release.
> + *
> + * The port reference does not pin @dport, which is a devm allocation of
> + * cxl_port_dport_host(). Pass NULL and use cxl_pci_find_dport() or
> + * cxl_mem_find_dport() under that device's lock instead.
>    */
>   static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
>   					       struct cxl_dport **dport)
> @@ -1943,6 +1947,38 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>   }
>   EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");
>   
> +/**
> + * cxl_pci_find_dport - find the dport of @port that @pdev is below
> + * @pdev: PCI device below the dport
> + * @port: port to search, typically from cxl_pci_find_port()
> + *
> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
> + *	    must not use the result after dropping it.
> + */
> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
> +				     struct cxl_port *port)
> +{
> +	device_lock_assert(cxl_port_dport_host(port));
> +	return cxl_find_dport_by_dev(port, pdev->dev.parent);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_pci_find_dport, "CXL");
> +
> +/**
> + * cxl_mem_find_dport - find the dport of @port that @cxlmd is below
> + * @cxlmd: memdev below the dport
> + * @port: port to search, typically from cxl_mem_find_port()
> + *
> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
> + *	    must not use the result after dropping it.
> + */
> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
> +				     struct cxl_port *port)
> +{
> +	device_lock_assert(cxl_port_dport_host(port));
> +	return cxl_find_dport_by_dev(port, grandparent(&cxlmd->dev));
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_find_dport, "CXL");
> +
>   static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
>   				    struct cxl_port *port)
>   {
> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> index e0e01aa5eba6..827a2f64d892 100644
> --- a/drivers/cxl/core/ras_rch.c
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -118,10 +118,20 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>   	int severity;
>   
>   	struct cxl_port *port __free(put_cxl_port) =
> -		cxl_pci_find_port(pdev, &dport);
> +		cxl_pci_find_port(pdev, NULL);
>   	if (!port)
>   		return;
>   
> +	/*
> +	 * The dport may have been freed since the lookup above, so take the
> +	 * lock that pins it and find it again.
> +	 */
> +	guard(device)(cxl_port_dport_host(port));
> +
> +	dport = cxl_pci_find_dport(pdev, port);
> +	if (!dport)
> +		return;
> +
>   	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>   		return;
>   
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index cab8ce39f465..f08de094f412 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -701,6 +701,18 @@ static inline bool is_cxl_root(struct cxl_port *port)
>   	return port->uport_dev == port->dev.parent;
>   }
>   
> +/*
> + * Return the owner device of the devm allocations of @port's dports. They
> + * are freed when that device unbinds, so hold device_lock() on the returned
> + * device across any lookup and use of a 'struct cxl_dport'.
> + */
> +static inline struct device *cxl_port_dport_host(struct cxl_port *port)
> +{
> +	if (is_cxl_root(port))
> +		return port->uport_dev;
> +	return &port->dev;
> +}
> +
>   /* Address translation functions exported to cxl_translate test module only */
>   int cxl_validate_translation_params(u8 eiw, u16 eig, int pos);
>   u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
> @@ -747,6 +759,10 @@ DEFINE_FREE(put_cxl_dax_region, struct cxl_dax_region *, if (!IS_ERR_OR_NULL(_T)
>   int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>   void cxl_bus_rescan(void);
>   void cxl_bus_drain(void);
> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
> +				     struct cxl_port *port);
> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
> +				     struct cxl_port *port);
>   struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
>   				   struct cxl_dport **dport);
>   struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 798e5c369cfc..91137487ac1f 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -133,7 +133,7 @@ static int cxl_mem_probe(struct device *dev)
>   		return rc;
>   
>   	struct cxl_port *parent_port __free(put_cxl_port) =
> -		cxl_mem_find_port(cxlmd, &dport);
> +		cxl_mem_find_port(cxlmd, NULL);
>   	if (!parent_port) {
>   		dev_err(dev, "CXL port topology not found\n");
>   		return -ENXIO;
> @@ -148,10 +148,7 @@ static int cxl_mem_probe(struct device *dev)
>   		}
>   	}
>   
> -	if (dport->rch)
> -		endpoint_parent = parent_port->uport_dev;
> -	else
> -		endpoint_parent = &parent_port->dev;
> +	endpoint_parent = cxl_port_dport_host(parent_port);
>   
>   	scoped_guard(device, endpoint_parent) {
>   		if (!endpoint_parent->driver) {
> @@ -160,6 +157,13 @@ static int cxl_mem_probe(struct device *dev)
>   			return -ENXIO;
>   		}
>   
> +		/* @dport is only valid while @endpoint_parent stays bound */
> +		dport = cxl_mem_find_dport(cxlmd, parent_port);
> +		if (!dport) {
> +			dev_err(dev, "CXL topology dport not found\n");
> +			return -ENXIO;
> +		}
> +
>   		rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
>   		if (rc)
>   			return rc;
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index c7c91e8dc51d..6e1d7ee94c19 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -706,22 +706,23 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size
>   	struct device *root_dev;
>   	struct cxl_dport *dport;
>   	struct cxl_port *root __free(put_cxl_port) =
> -		cxl_mem_find_port(cxlmd, &dport);
> +		cxl_mem_find_port(cxlmd, NULL);
>   
>   	if (!root)
>   		return -ENXIO;
>   
> -	root_dev = root->uport_dev;
> +	root_dev = cxl_port_dport_host(root);
>   	if (!root_dev)
>   		return -ENXIO;
>   
> -	if (!dport->regs.rcd_pcie_cap)
> -		return -ENXIO;
> -
>   	guard(device)(root_dev);
>   	if (!root_dev->driver)
>   		return -ENXIO;
>   
> +	dport = cxl_mem_find_dport(cxlmd, root);
> +	if (!dport || !dport->regs.rcd_pcie_cap)
> +		return -ENXIO;
> +
>   	switch (width) {
>   	case 2:
>   		return sysfs_emit(buf, "%#x\n",

Re: [PATCH v2 2/2] cxl/core: Hold the dport host lock across dport lookup and use

[Dave Jiang]

On 9/29/26 2:21 AM, Li Ming wrote:
> 
> 在 2026/9/29 07:03, Dave Jiang 写道:
>> Callers of cxl_pci_find_port() and cxl_mem_find_port() can be handed a
>> 'struct cxl_dport' that is already freed, and all four dereference it
>> without the lock that would prevent that.
>>
>> A dport is kzalloc()'d into a devres group on its host device, so it dies
>> when that host unbinds. The host is &port->dev for a switch port and
>> port->uport_dev for the root port.
> 
> In unbinding case, seems like CXL driver needs to check port's driver before accessing a dport of the port. I guess the dport in devm_cxl_enumerate_ports() and add_port_attach_ep() also needs to be protected by port's lock? Like cxl_add_ep() in devm_cxl_enumerate_ports(), it is invoked without port's lock, the dport could be freed during the function calling.

I'll take a look. 


> 
> BTW, just wondering whether adding a dedicated "struct device" to dport makes more sense? use get_device()/put_device() to prevent UAF of dports.

Do you think using the dport->dport_dev is not sufficient? I fear how much messier the whole thing would be if we add a 'struct device' to dport now.

DJ

> 
>>
>> Both helpers return the dport through an output parameter. The caller's
>> port reference pins struct cxl_port, not the dport allocation, and the
>> lookup itself is a bare xa_load() under no lock.
>>
>> Add cxl_port_dport_host() to name the device that pins a dport, and
>> cxl_pci_find_dport() / cxl_mem_find_dport() to look one up with that
>> device's lock held and asserted. Convert the four callers to use them and
>> bail if the dport is gone. cxl_port_add_dport() already follows this rule.
>>
>> cxl_mem_probe() chose its endpoint devm host from dport->rch, which meant
>> dereferencing the dport before taking any lock. Use cxl_port_dport_host():
>> rch dports only ever land on the root port, so it selects the same device
>> without needing the dport.
>>
>> Fixes: 733b57f262b0 ("cxl/pci: Early setup RCH dport component registers from RCRB")
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> Assisted-by: LLM
>> ---
>> v2:
>> - Reword the cxl_port_dport_host() comment. (Jonathan)
>> - Drop the note on stale cxlsd->target[] pointers, fixed by patch 1. (Jonathan)
>>
>> Found by review, not from a failure report.
>>
>> Untested: two of the four converted sites add a new lock acquisition, and
>> neither executed under test.
>>
>>    cxl_pci_setup_regs()        new guard() sits behind is_cxl_restricted(),
>>                                which needs a PCI_EXP_TYPE_RC_END device
>>    cxl_handle_rdport_errors()  needs an RCH dport AER error; cxl_test has no
>>                                injection path
>>
>> Only cxl_pci_setup_regs() introduces a lock order lockdep has not already
>> seen: device_lock(&pdev->dev) from probe, then the dport host lock.
>>
>> cxl_handle_rdport_errors() nests its new host lock inside the
>> device_lock(&cxlmd->dev) that core/ras.c:274 and :298 hold, but
>> cxl_mem_probe() takes that same pair in that same order and ran clean under
>> PROVE_LOCKING, so the edge itself is covered. What is untested there is the
>> path, not the ordering.
>>
>> Reaching either needs a cxl-type3 attached directly to a host bridge so
>> pci_pcie_type() is PCI_EXP_TYPE_RC_END, which run_qemu.sh does not generate.
>>
>> Tested in QEMU with cxl_test, PROVE_LOCKING and KASAN on, 8 module
>> load/unload cycles with concurrent sysfs readers. No lockdep splat, no
>> device_lock_assert() firing, enumeration unchanged (15 memdev, 12 port,
>> 192 decoder before and after).
>>
>> cxl_mem_find_dport() ran for 15 memdevs on each of 9 loads, covering both
>> cxl_port_dport_host() branches. The rch branch is covered too: "cxl_mem
>> mem2: endpoint9 added to root3" shows the new selection picking the same
>> parent dport->rch did.
>> ---
>>   drivers/cxl/core/core.h    |  6 +-----
>>   drivers/cxl/core/pci.c     |  8 +++++++-
>>   drivers/cxl/core/port.c    | 36 ++++++++++++++++++++++++++++++++++++
>>   drivers/cxl/core/ras_rch.c | 12 +++++++++++-
>>   drivers/cxl/cxl.h          | 16 ++++++++++++++++
>>   drivers/cxl/mem.c          | 14 +++++++++-----
>>   drivers/cxl/pci.c          | 11 ++++++-----
>>   7 files changed, 86 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
>> index 35eaf636adc9..0a39e500029a 100644
>> --- a/drivers/cxl/core/core.h
>> +++ b/drivers/cxl/core/core.h
>> @@ -178,11 +178,7 @@ static inline struct device *port_to_host(struct cxl_port *port)
>>     static inline struct device *dport_to_host(struct cxl_dport *dport)
>>   {
>> -    struct cxl_port *port = dport->port;
>> -
>> -    if (is_cxl_root(port))
>> -        return port->uport_dev;
>> -    return &port->dev;
>> +    return cxl_port_dport_host(dport->port);
>>   }
>>   #ifdef CONFIG_CXL_RAS
>>   void cxl_ras_init(void);
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 9d807c1a002c..8a236e22bb3f 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -734,10 +734,16 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>>       if (rc && type == CXL_REGLOC_RBI_COMPONENT && is_cxl_restricted(pdev)) {
>>           struct cxl_dport *dport;
>>           struct cxl_port *port __free(put_cxl_port) =
>> -            cxl_pci_find_port(pdev, &dport);
>> +            cxl_pci_find_port(pdev, NULL);
>>           if (!port)
>>               return -EPROBE_DEFER;
>>   +        guard(device)(cxl_port_dport_host(port));
>> +
>> +        dport = cxl_pci_find_dport(pdev, port);
>> +        if (!dport)
>> +            return -EPROBE_DEFER;
>> +
>>           rc = cxl_rcrb_get_comp_regs(pdev, map, dport);
>>           if (rc)
>>               return rc;
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 6ff3353865e3..336d7c0f5d98 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1428,6 +1428,10 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
>>    *
>>    * Return a 'struct cxl_port' with an elevated reference if found. Use
>>    * __free(put_cxl_port) to release.
>> + *
>> + * The port reference does not pin @dport, which is a devm allocation of
>> + * cxl_port_dport_host(). Pass NULL and use cxl_pci_find_dport() or
>> + * cxl_mem_find_dport() under that device's lock instead.
>>    */
>>   static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
>>                              struct cxl_dport **dport)
>> @@ -1943,6 +1947,38 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");
>>   +/**
>> + * cxl_pci_find_dport - find the dport of @port that @pdev is below
>> + * @pdev: PCI device below the dport
>> + * @port: port to search, typically from cxl_pci_find_port()
>> + *
>> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
>> + *        must not use the result after dropping it.
>> + */
>> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
>> +                     struct cxl_port *port)
>> +{
>> +    device_lock_assert(cxl_port_dport_host(port));
>> +    return cxl_find_dport_by_dev(port, pdev->dev.parent);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_pci_find_dport, "CXL");
>> +
>> +/**
>> + * cxl_mem_find_dport - find the dport of @port that @cxlmd is below
>> + * @cxlmd: memdev below the dport
>> + * @port: port to search, typically from cxl_mem_find_port()
>> + *
>> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
>> + *        must not use the result after dropping it.
>> + */
>> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
>> +                     struct cxl_port *port)
>> +{
>> +    device_lock_assert(cxl_port_dport_host(port));
>> +    return cxl_find_dport_by_dev(port, grandparent(&cxlmd->dev));
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_mem_find_dport, "CXL");
>> +
>>   static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
>>                       struct cxl_port *port)
>>   {
>> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
>> index e0e01aa5eba6..827a2f64d892 100644
>> --- a/drivers/cxl/core/ras_rch.c
>> +++ b/drivers/cxl/core/ras_rch.c
>> @@ -118,10 +118,20 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>>       int severity;
>>         struct cxl_port *port __free(put_cxl_port) =
>> -        cxl_pci_find_port(pdev, &dport);
>> +        cxl_pci_find_port(pdev, NULL);
>>       if (!port)
>>           return;
>>   +    /*
>> +     * The dport may have been freed since the lookup above, so take the
>> +     * lock that pins it and find it again.
>> +     */
>> +    guard(device)(cxl_port_dport_host(port));
>> +
>> +    dport = cxl_pci_find_dport(pdev, port);
>> +    if (!dport)
>> +        return;
>> +
>>       if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>>           return;
>>   diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index cab8ce39f465..f08de094f412 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -701,6 +701,18 @@ static inline bool is_cxl_root(struct cxl_port *port)
>>       return port->uport_dev == port->dev.parent;
>>   }
>>   +/*
>> + * Return the owner device of the devm allocations of @port's dports. They
>> + * are freed when that device unbinds, so hold device_lock() on the returned
>> + * device across any lookup and use of a 'struct cxl_dport'.
>> + */
>> +static inline struct device *cxl_port_dport_host(struct cxl_port *port)
>> +{
>> +    if (is_cxl_root(port))
>> +        return port->uport_dev;
>> +    return &port->dev;
>> +}
>> +
>>   /* Address translation functions exported to cxl_translate test module only */
>>   int cxl_validate_translation_params(u8 eiw, u16 eig, int pos);
>>   u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
>> @@ -747,6 +759,10 @@ DEFINE_FREE(put_cxl_dax_region, struct cxl_dax_region *, if (!IS_ERR_OR_NULL(_T)
>>   int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>>   void cxl_bus_rescan(void);
>>   void cxl_bus_drain(void);
>> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
>> +                     struct cxl_port *port);
>> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
>> +                     struct cxl_port *port);
>>   struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
>>                      struct cxl_dport **dport);
>>   struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 798e5c369cfc..91137487ac1f 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -133,7 +133,7 @@ static int cxl_mem_probe(struct device *dev)
>>           return rc;
>>         struct cxl_port *parent_port __free(put_cxl_port) =
>> -        cxl_mem_find_port(cxlmd, &dport);
>> +        cxl_mem_find_port(cxlmd, NULL);
>>       if (!parent_port) {
>>           dev_err(dev, "CXL port topology not found\n");
>>           return -ENXIO;
>> @@ -148,10 +148,7 @@ static int cxl_mem_probe(struct device *dev)
>>           }
>>       }
>>   -    if (dport->rch)
>> -        endpoint_parent = parent_port->uport_dev;
>> -    else
>> -        endpoint_parent = &parent_port->dev;
>> +    endpoint_parent = cxl_port_dport_host(parent_port);
>>         scoped_guard(device, endpoint_parent) {
>>           if (!endpoint_parent->driver) {
>> @@ -160,6 +157,13 @@ static int cxl_mem_probe(struct device *dev)
>>               return -ENXIO;
>>           }
>>   +        /* @dport is only valid while @endpoint_parent stays bound */
>> +        dport = cxl_mem_find_dport(cxlmd, parent_port);
>> +        if (!dport) {
>> +            dev_err(dev, "CXL topology dport not found\n");
>> +            return -ENXIO;
>> +        }
>> +
>>           rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
>>           if (rc)
>>               return rc;
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index c7c91e8dc51d..6e1d7ee94c19 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -706,22 +706,23 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size
>>       struct device *root_dev;
>>       struct cxl_dport *dport;
>>       struct cxl_port *root __free(put_cxl_port) =
>> -        cxl_mem_find_port(cxlmd, &dport);
>> +        cxl_mem_find_port(cxlmd, NULL);
>>         if (!root)
>>           return -ENXIO;
>>   -    root_dev = root->uport_dev;
>> +    root_dev = cxl_port_dport_host(root);
>>       if (!root_dev)
>>           return -ENXIO;
>>   -    if (!dport->regs.rcd_pcie_cap)
>> -        return -ENXIO;
>> -
>>       guard(device)(root_dev);
>>       if (!root_dev->driver)
>>           return -ENXIO;
>>   +    dport = cxl_mem_find_dport(cxlmd, root);
>> +    if (!dport || !dport->regs.rcd_pcie_cap)
>> +        return -ENXIO;
>> +
>>       switch (width) {
>>       case 2:
>>           return sysfs_emit(buf, "%#x\n",

Re: [PATCH v2 2/2] cxl/core: Hold the dport host lock across dport lookup and use

[Li Ming]

On 9/29/2026 11:57 PM, Dave Jiang wrote:
>
> On 9/29/26 2:21 AM, Li Ming wrote:
>> 在 2026/9/29 07:03, Dave Jiang 写道:
>>> Callers of cxl_pci_find_port() and cxl_mem_find_port() can be handed a
>>> 'struct cxl_dport' that is already freed, and all four dereference it
>>> without the lock that would prevent that.
>>>
>>> A dport is kzalloc()'d into a devres group on its host device, so it dies
>>> when that host unbinds. The host is &port->dev for a switch port and
>>> port->uport_dev for the root port.
>> In unbinding case, seems like CXL driver needs to check port's driver before accessing a dport of the port. I guess the dport in devm_cxl_enumerate_ports() and add_port_attach_ep() also needs to be protected by port's lock? Like cxl_add_ep() in devm_cxl_enumerate_ports(), it is invoked without port's lock, the dport could be freed during the function calling.
> I'll take a look. 
>
>
>> BTW, just wondering whether adding a dedicated "struct device" to dport makes more sense? use get_device()/put_device() to prevent UAF of dports.
> Do you think using the dport->dport_dev is not sufficient? I fear how much messier the whole thing would be if we add a 'struct device' to dport now.

This is only a rough idea. Currently the dport lifetime is tied to the port driver lifetime. It seems unreasonable that dport gets released when the port unbinds from its driver. My feeling is that the dport lifetime should be independent of the port driver, since dport acts more like a standalone device under the port. Admittedly this would require a large amount of changes.

Maybe we can release the dports at port release time if you also agree that dports should not be released upon port driver unbind. 

In any case, there are just my thoughts. The current implementation works fine for me.


Ming

>
> DJ
>
>>> Both helpers return the dport through an output parameter. The caller's
>>> port reference pins struct cxl_port, not the dport allocation, and the
>>> lookup itself is a bare xa_load() under no lock.
>>>
>>> Add cxl_port_dport_host() to name the device that pins a dport, and
>>> cxl_pci_find_dport() / cxl_mem_find_dport() to look one up with that
>>> device's lock held and asserted. Convert the four callers to use them and
>>> bail if the dport is gone. cxl_port_add_dport() already follows this rule.
>>>
>>> cxl_mem_probe() chose its endpoint devm host from dport->rch, which meant
>>> dereferencing the dport before taking any lock. Use cxl_port_dport_host():
>>> rch dports only ever land on the root port, so it selects the same device
>>> without needing the dport.
>>>
>>> Fixes: 733b57f262b0 ("cxl/pci: Early setup RCH dport component registers from RCRB")
>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>> Assisted-by: LLM
>>> ---
>>> v2:
>>> - Reword the cxl_port_dport_host() comment. (Jonathan)
>>> - Drop the note on stale cxlsd->target[] pointers, fixed by patch 1. (Jonathan)
>>>
>>> Found by review, not from a failure report.
>>>
>>> Untested: two of the four converted sites add a new lock acquisition, and
>>> neither executed under test.
>>>
>>>    cxl_pci_setup_regs()        new guard() sits behind is_cxl_restricted(),
>>>                                which needs a PCI_EXP_TYPE_RC_END device
>>>    cxl_handle_rdport_errors()  needs an RCH dport AER error; cxl_test has no
>>>                                injection path
>>>
>>> Only cxl_pci_setup_regs() introduces a lock order lockdep has not already
>>> seen: device_lock(&pdev->dev) from probe, then the dport host lock.
>>>
>>> cxl_handle_rdport_errors() nests its new host lock inside the
>>> device_lock(&cxlmd->dev) that core/ras.c:274 and :298 hold, but
>>> cxl_mem_probe() takes that same pair in that same order and ran clean under
>>> PROVE_LOCKING, so the edge itself is covered. What is untested there is the
>>> path, not the ordering.
>>>
>>> Reaching either needs a cxl-type3 attached directly to a host bridge so
>>> pci_pcie_type() is PCI_EXP_TYPE_RC_END, which run_qemu.sh does not generate.
>>>
>>> Tested in QEMU with cxl_test, PROVE_LOCKING and KASAN on, 8 module
>>> load/unload cycles with concurrent sysfs readers. No lockdep splat, no
>>> device_lock_assert() firing, enumeration unchanged (15 memdev, 12 port,
>>> 192 decoder before and after).
>>>
>>> cxl_mem_find_dport() ran for 15 memdevs on each of 9 loads, covering both
>>> cxl_port_dport_host() branches. The rch branch is covered too: "cxl_mem
>>> mem2: endpoint9 added to root3" shows the new selection picking the same
>>> parent dport->rch did.
>>> ---
>>>   drivers/cxl/core/core.h    |  6 +-----
>>>   drivers/cxl/core/pci.c     |  8 +++++++-
>>>   drivers/cxl/core/port.c    | 36 ++++++++++++++++++++++++++++++++++++
>>>   drivers/cxl/core/ras_rch.c | 12 +++++++++++-
>>>   drivers/cxl/cxl.h          | 16 ++++++++++++++++
>>>   drivers/cxl/mem.c          | 14 +++++++++-----
>>>   drivers/cxl/pci.c          | 11 ++++++-----
>>>   7 files changed, 86 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
>>> index 35eaf636adc9..0a39e500029a 100644
>>> --- a/drivers/cxl/core/core.h
>>> +++ b/drivers/cxl/core/core.h
>>> @@ -178,11 +178,7 @@ static inline struct device *port_to_host(struct cxl_port *port)
>>>     static inline struct device *dport_to_host(struct cxl_dport *dport)
>>>   {
>>> -    struct cxl_port *port = dport->port;
>>> -
>>> -    if (is_cxl_root(port))
>>> -        return port->uport_dev;
>>> -    return &port->dev;
>>> +    return cxl_port_dport_host(dport->port);
>>>   }
>>>   #ifdef CONFIG_CXL_RAS
>>>   void cxl_ras_init(void);
>>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>>> index 9d807c1a002c..8a236e22bb3f 100644
>>> --- a/drivers/cxl/core/pci.c
>>> +++ b/drivers/cxl/core/pci.c
>>> @@ -734,10 +734,16 @@ int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>>>       if (rc && type == CXL_REGLOC_RBI_COMPONENT && is_cxl_restricted(pdev)) {
>>>           struct cxl_dport *dport;
>>>           struct cxl_port *port __free(put_cxl_port) =
>>> -            cxl_pci_find_port(pdev, &dport);
>>> +            cxl_pci_find_port(pdev, NULL);
>>>           if (!port)
>>>               return -EPROBE_DEFER;
>>>   +        guard(device)(cxl_port_dport_host(port));
>>> +
>>> +        dport = cxl_pci_find_dport(pdev, port);
>>> +        if (!dport)
>>> +            return -EPROBE_DEFER;
>>> +
>>>           rc = cxl_rcrb_get_comp_regs(pdev, map, dport);
>>>           if (rc)
>>>               return rc;
>>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>>> index 6ff3353865e3..336d7c0f5d98 100644
>>> --- a/drivers/cxl/core/port.c
>>> +++ b/drivers/cxl/core/port.c
>>> @@ -1428,6 +1428,10 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
>>>    *
>>>    * Return a 'struct cxl_port' with an elevated reference if found. Use
>>>    * __free(put_cxl_port) to release.
>>> + *
>>> + * The port reference does not pin @dport, which is a devm allocation of
>>> + * cxl_port_dport_host(). Pass NULL and use cxl_pci_find_dport() or
>>> + * cxl_mem_find_dport() under that device's lock instead.
>>>    */
>>>   static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
>>>                              struct cxl_dport **dport)
>>> @@ -1943,6 +1947,38 @@ struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>>>   }
>>>   EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");
>>>   +/**
>>> + * cxl_pci_find_dport - find the dport of @port that @pdev is below
>>> + * @pdev: PCI device below the dport
>>> + * @port: port to search, typically from cxl_pci_find_port()
>>> + *
>>> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
>>> + *        must not use the result after dropping it.
>>> + */
>>> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
>>> +                     struct cxl_port *port)
>>> +{
>>> +    device_lock_assert(cxl_port_dport_host(port));
>>> +    return cxl_find_dport_by_dev(port, pdev->dev.parent);
>>> +}
>>> +EXPORT_SYMBOL_NS_GPL(cxl_pci_find_dport, "CXL");
>>> +
>>> +/**
>>> + * cxl_mem_find_dport - find the dport of @port that @cxlmd is below
>>> + * @cxlmd: memdev below the dport
>>> + * @port: port to search, typically from cxl_mem_find_port()
>>> + *
>>> + * Context: Caller must hold the cxl_port_dport_host() lock of @port, and
>>> + *        must not use the result after dropping it.
>>> + */
>>> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
>>> +                     struct cxl_port *port)
>>> +{
>>> +    device_lock_assert(cxl_port_dport_host(port));
>>> +    return cxl_find_dport_by_dev(port, grandparent(&cxlmd->dev));
>>> +}
>>> +EXPORT_SYMBOL_NS_GPL(cxl_mem_find_dport, "CXL");
>>> +
>>>   static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd,
>>>                       struct cxl_port *port)
>>>   {
>>> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
>>> index e0e01aa5eba6..827a2f64d892 100644
>>> --- a/drivers/cxl/core/ras_rch.c
>>> +++ b/drivers/cxl/core/ras_rch.c
>>> @@ -118,10 +118,20 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>>>       int severity;
>>>         struct cxl_port *port __free(put_cxl_port) =
>>> -        cxl_pci_find_port(pdev, &dport);
>>> +        cxl_pci_find_port(pdev, NULL);
>>>       if (!port)
>>>           return;
>>>   +    /*
>>> +     * The dport may have been freed since the lookup above, so take the
>>> +     * lock that pins it and find it again.
>>> +     */
>>> +    guard(device)(cxl_port_dport_host(port));
>>> +
>>> +    dport = cxl_pci_find_dport(pdev, port);
>>> +    if (!dport)
>>> +        return;
>>> +
>>>       if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>>>           return;
>>>   diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>>> index cab8ce39f465..f08de094f412 100644
>>> --- a/drivers/cxl/cxl.h
>>> +++ b/drivers/cxl/cxl.h
>>> @@ -701,6 +701,18 @@ static inline bool is_cxl_root(struct cxl_port *port)
>>>       return port->uport_dev == port->dev.parent;
>>>   }
>>>   +/*
>>> + * Return the owner device of the devm allocations of @port's dports. They
>>> + * are freed when that device unbinds, so hold device_lock() on the returned
>>> + * device across any lookup and use of a 'struct cxl_dport'.
>>> + */
>>> +static inline struct device *cxl_port_dport_host(struct cxl_port *port)
>>> +{
>>> +    if (is_cxl_root(port))
>>> +        return port->uport_dev;
>>> +    return &port->dev;
>>> +}
>>> +
>>>   /* Address translation functions exported to cxl_translate test module only */
>>>   int cxl_validate_translation_params(u8 eiw, u16 eig, int pos);
>>>   u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
>>> @@ -747,6 +759,10 @@ DEFINE_FREE(put_cxl_dax_region, struct cxl_dax_region *, if (!IS_ERR_OR_NULL(_T)
>>>   int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>>>   void cxl_bus_rescan(void);
>>>   void cxl_bus_drain(void);
>>> +struct cxl_dport *cxl_pci_find_dport(struct pci_dev *pdev,
>>> +                     struct cxl_port *port);
>>> +struct cxl_dport *cxl_mem_find_dport(struct cxl_memdev *cxlmd,
>>> +                     struct cxl_port *port);
>>>   struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
>>>                      struct cxl_dport **dport);
>>>   struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
>>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>>> index 798e5c369cfc..91137487ac1f 100644
>>> --- a/drivers/cxl/mem.c
>>> +++ b/drivers/cxl/mem.c
>>> @@ -133,7 +133,7 @@ static int cxl_mem_probe(struct device *dev)
>>>           return rc;
>>>         struct cxl_port *parent_port __free(put_cxl_port) =
>>> -        cxl_mem_find_port(cxlmd, &dport);
>>> +        cxl_mem_find_port(cxlmd, NULL);
>>>       if (!parent_port) {
>>>           dev_err(dev, "CXL port topology not found\n");
>>>           return -ENXIO;
>>> @@ -148,10 +148,7 @@ static int cxl_mem_probe(struct device *dev)
>>>           }
>>>       }
>>>   -    if (dport->rch)
>>> -        endpoint_parent = parent_port->uport_dev;
>>> -    else
>>> -        endpoint_parent = &parent_port->dev;
>>> +    endpoint_parent = cxl_port_dport_host(parent_port);
>>>         scoped_guard(device, endpoint_parent) {
>>>           if (!endpoint_parent->driver) {
>>> @@ -160,6 +157,13 @@ static int cxl_mem_probe(struct device *dev)
>>>               return -ENXIO;
>>>           }
>>>   +        /* @dport is only valid while @endpoint_parent stays bound */
>>> +        dport = cxl_mem_find_dport(cxlmd, parent_port);
>>> +        if (!dport) {
>>> +            dev_err(dev, "CXL topology dport not found\n");
>>> +            return -ENXIO;
>>> +        }
>>> +
>>>           rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
>>>           if (rc)
>>>               return rc;
>>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>>> index c7c91e8dc51d..6e1d7ee94c19 100644
>>> --- a/drivers/cxl/pci.c
>>> +++ b/drivers/cxl/pci.c
>>> @@ -706,22 +706,23 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size
>>>       struct device *root_dev;
>>>       struct cxl_dport *dport;
>>>       struct cxl_port *root __free(put_cxl_port) =
>>> -        cxl_mem_find_port(cxlmd, &dport);
>>> +        cxl_mem_find_port(cxlmd, NULL);
>>>         if (!root)
>>>           return -ENXIO;
>>>   -    root_dev = root->uport_dev;
>>> +    root_dev = cxl_port_dport_host(root);
>>>       if (!root_dev)
>>>           return -ENXIO;
>>>   -    if (!dport->regs.rcd_pcie_cap)
>>> -        return -ENXIO;
>>> -
>>>       guard(device)(root_dev);
>>>       if (!root_dev->driver)
>>>           return -ENXIO;
>>>   +    dport = cxl_mem_find_dport(cxlmd, root);
>>> +    if (!dport || !dport->regs.rcd_pcie_cap)
>>> +        return -ENXIO;
>>> +
>>>       switch (width) {
>>>       case 2:
>>>           return sysfs_emit(buf, "%#x\n",