Skip to contentSkip to conversation

cxl/mem: Fix no cxl_nvd during pmem region auto-assembling

PATCH Applied Artem Dinaburg 3 participants 3 messages Sep 29 Stable branch

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cxl/mem: Fix no cxl_nvd during pmem region auto-assembling

From: Li Ming <[email protected]>

[ Upstream commit 84ec985944ef34a34a1605b93ce401aa8737af96 ]

When CXL subsystem is auto-assembling a pmem region during cxl endpoint port probing, always hit below calltrace.

 BUG: kernel NULL pointer dereference, address: 0000000000000078
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 RIP: 0010:cxl_pmem_region_probe+0x22e/0x360 [cxl_pmem]
 Call Trace:
  <TASK>
  ? __die+0x24/0x70
  ? page_fault_oops+0x82/0x160
  ? do_user_addr_fault+0x65/0x6b0
  ? exc_page_fault+0x7d/0x170
  ? asm_exc_page_fault+0x26/0x30
  ? cxl_pmem_region_probe+0x22e/0x360 [cxl_pmem]
  ? cxl_pmem_region_probe+0x1ac/0x360 [cxl_pmem]
  cxl_bus_probe+0x1b/0x60 [cxl_core]
  really_probe+0x173/0x410
  ? __pfx___device_attach_driver+0x10/0x10
  __driver_probe_device+0x80/0x170
  driver_probe_device+0x1e/0x90
  __device_attach_driver+0x90/0x120
  bus_for_each_drv+0x84/0xe0
  __device_attach+0xbc/0x1f0
  bus_probe_device+0x90/0xa0
  device_add+0x51c/0x710
  devm_cxl_add_pmem_region+0x1b5/0x380 [cxl_core]
  cxl_bus_probe+0x1b/0x60 [cxl_core]

The cxl_nvd of the memdev needs to be available during the pmem region probe. Currently the cxl_nvd is registered after the endpoint port probe. The endpoint probe, in the case of autoassembly of regions, can cause a pmem region probe requiring the not yet available cxl_nvd. Adjust the sequence so this dependency is met.

This requires adding a port parameter to cxl_find_nvdimm_bridge() that can be used to query the ancestor root port. The endpoint port is not yet available, but will share a common ancestor with its parent, so start the query from there instead.

[ Backport to 6.6.y: mapped find_cxl_root() and reference handling to the
older struct cxl_port and put_device() APIs. The new early error return explicitly drops parent_port because 6.6.y lacks the scoped cleanup later added by upstream commit dd2617ebd2a6 ("cxl/port: Use __free() to drop put_device() for cxl_port"). ]

Fixes f17b558d6663 ("cxl/pmem: Refactor nvdimm device registration, delete the workqueue")
Assisted-by LLM

---
Hi Greg, Sasha, and cxl maintainers,

I am working through the small CVE backports still missing from 6.6.y. This one addresses CVE-2024-41085. It registers the nvdimm bridge before endpoint probing can auto-assemble a pmem region.

The fix is already present in 6.12.y, 6.18.y, and 7.2.y, but not in 6.6.y. The target-specific adjustment is recorded in the bracketed note above. The added error-path put_device() matches the cleanup that current mainline performs automatically through its scoped cxl_port reference.

Could you please queue it for 6.6.y?

CVE: CVE-2024-41085
Upstream: 84ec985944ef34a34a1605b93ce401aa8737af96

AI assistance: An LLM helped identify, adapt, and validate this backport; I reviewed the resulting code and validation evidence.

Thanks,
Artem Dinaburg

 drivers/cxl/core/pmem.c   | 21 ++++++++++++++-------
 drivers/cxl/core/region.c |  2 +-
 drivers/cxl/cxl.h         |  4 ++--
 drivers/cxl/mem.c         | 18 ++++++++++--------
 4 files changed, 27 insertions(+), 18 deletions(-)
drivers/cxl/core/pmem.c +14 -7
index 317b2fbbef668f..1f87ce9191a74d 100644
@@ -62,16 +62,21 @@ static int match_nvdimm_bridge(struct device *dev, void *data)
 	return is_cxl_nvdimm_bridge(dev);
 }
 
-struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_memdev *cxlmd)
+/**
+ * cxl_find_nvdimm_bridge() - find a bridge device relative to a port
+ * @port: any descendant port of an nvdimm-bridge associated
+ *        root-cxl-port
+ */
+struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port)
 {
-	struct cxl_port *port = find_cxl_root(cxlmd->endpoint);
+	struct cxl_port *root = find_cxl_root(port);
 	struct device *dev;
 
-	if (!port)
+	if (!root)
 		return NULL;
 
-	dev = device_find_child(&port->dev, NULL, match_nvdimm_bridge);
-	put_device(&port->dev);
+	dev = device_find_child(&root->dev, NULL, match_nvdimm_bridge);
+	put_device(&root->dev);
 
 	if (!dev)
 		return NULL;
@@ -244,18 +249,20 @@ static void cxlmd_release_nvdimm(void *_cxlmd)
 
 /**
  * devm_cxl_add_nvdimm() - add a bridge between a cxl_memdev and an nvdimm
+ * @parent_port: parent port for the (to be added) @cxlmd endpoint port
  * @cxlmd: cxl_memdev instance that will perform LIBNVDIMM operations
  *
  * Return: 0 on success negative error code on failure.
  */
-int devm_cxl_add_nvdimm(struct cxl_memdev *cxlmd)
+int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
+			struct cxl_memdev *cxlmd)
 {
 	struct cxl_nvdimm_bridge *cxl_nvb;
 	struct cxl_nvdimm *cxl_nvd;
 	struct device *dev;
 	int rc;
 
-	cxl_nvb = cxl_find_nvdimm_bridge(cxlmd);
+	cxl_nvb = cxl_find_nvdimm_bridge(parent_port);
 	if (!cxl_nvb)
 		return -ENODEV;
 
drivers/cxl/core/region.c +1 -1
index af4f8bbe63c448..97e4a4755ab2f8 100644
@@ -2682,7 +2682,7 @@ static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
 		 * bridge for one device is the same for all.
 		 */
 		if (i == 0) {
-			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd);
+			cxl_nvb = cxl_find_nvdimm_bridge(cxlmd->endpoint);
 			if (!cxl_nvb) {
 				kfree(cxlr_pmem);
 				cxlr_pmem = ERR_PTR(-ENODEV);
drivers/cxl/cxl.h +2 -2
index a7a1b71626de43..59e963e39d31b1 100644
@@ -795,8 +795,8 @@ struct cxl_nvdimm_bridge *devm_cxl_add_nvdimm_bridge(struct device *host,
 struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm_bridge(struct device *dev);
-int devm_cxl_add_nvdimm(struct cxl_memdev *cxlmd);
-struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_memdev *cxlmd);
+int devm_cxl_add_nvdimm(struct cxl_port *parent_port, struct cxl_memdev *cxlmd);
+struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port);
 
 #ifdef CONFIG_CXL_REGION
 bool is_cxl_pmem_region(struct device *dev);
drivers/cxl/mem.c +10 -8
index 317c7548e4e9b4..3400d2f2a5ff92 100644
@@ -153,6 +153,16 @@ static int cxl_mem_probe(struct device *dev)
 		return -ENXIO;
 	}
 
+	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
+		rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
+		if (rc) {
+			if (rc == -ENODEV)
+				dev_info(dev, "PMEM disabled by platform\n");
+			put_device(&parent_port->dev);
+			return rc;
+		}
+	}
+
 	if (dport->rch)
 		endpoint_parent = parent_port->uport_dev;
 	else
@@ -173,14 +183,6 @@ static int cxl_mem_probe(struct device *dev)
 	if (rc)
 		return rc;
 
-	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
-		rc = devm_cxl_add_nvdimm(cxlmd);
-		if (rc == -ENODEV)
-			dev_info(dev, "PMEM disabled by platform\n");
-		else
-			return rc;
-	}
-
 	/*
 	 * The kernel may be operating out of CXL memory on this device,
 	 * there is no spec defined way to determine whether this device
Signature · 1 line
2.39.5
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] cxl: fatal probe failure when PMEM is disabled by platform --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl: fatal probe failure when PMEM is disabled by platform
--

commit 3836a307f91efffcbe3c18ffddeb397cd4ef78b8
Author: Li Ming <[email protected]>

cxl/mem: Fix no cxl_nvd during pmem region auto-assembling

When the CXL subsystem is auto-assembling a pmem region during cxl endpoint port probing, it can hit a null pointer dereference. The cxl_nvd of the memdev needs to be available during the pmem region probe, so this patch adjusts the sequence to register the nvdimm bridge earlier.

Quoted 5 lines
drivers/cxl/mem.c +0 -0
index 317c7548e4e9b..3400d2f2a5ff9 100644

drivers/cxl/mem.c static int cxl_mem_probe(struct device *dev) · line 162

Quoted 10 lines
drivers/cxl/mem.c +7 -0
@@ -153,6 +153,16 @@ static int cxl_mem_probe(struct device *dev)
 		return -ENXIO;
 	}
 
+	if (resource_size(&cxlds->pmem_res) && IS_ENABLED(CONFIG_CXL_PMEM)) {
+		rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
+		if (rc) {
+			if (rc == -ENODEV)
+				dev_info(dev, "PMEM disabled by platform\n");
+			put_device(&parent_port->dev);
+			return rc;

[Severity: High]
Can returning an error here inadvertently cause a fatal probe failure when the platform disables PMEM?

If a CXL memory expander has a PMEM region but the platform lacks an NVDIMM bridge, devm_cxl_add_nvdimm() correctly returns -ENODEV.

The new code catches this and logs "PMEM disabled by platform", but then it unconditionally returns rc. Returning an error here fails the entire cxl_mem driver probe, meaning the device is rejected and its endpoint is never registered.

This renders the device completely unusable, blocking access to both its persistent and volatile memory. The old code correctly swallowed the -ENODEV error to allow continued probing.

Quoted 5 lines
drivers/cxl/mem.c +3 -0
+		}
+	}
+
 	if (dport->rch)
 		endpoint_parent = parent_port->uport_dev;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer Applied
#2 lore raw
In reply to Artem Dinaburg
Quoted 2 lines

This one addresses CVE-2024-41085. It registers the nvdimm bridge before endpoint probing can auto-assemble a pmem region.

Queued for 6.6, thanks.

Signature · 2 lines
Thanks,
Sasha