Skip to content

Commit 8d2ad99

Browse files
committed
cxl/port: Fix delete_endpoint() vs parent unregistration race
The CXL subsystem, at cxl_mem ->probe() time, establishes a lineage of ports (struct cxl_port objects) between an endpoint and the root of a CXL topology. Each port including the endpoint port is attached to the cxl_port driver. Given that setup, it follows that when either any port in that lineage goes through a cxl_port ->remove() event, or the memdev goes through a cxl_mem ->remove() event. The hierarchy below the removed port, or the entire hierarchy if the memdev is removed needs to come down. The delete_endpoint() callback is careful to check whether it is being called to tear down the hierarchy, or if it is only being called to teardown the memdev because an ancestor port is going through ->remove(). That care needs to take the device_lock() of the endpoint's parent. Which requires 2 bugs to be fixed: 1/ A reference on the parent is needed to prevent use-after-free scenarios like this signature: BUG: spinlock bad magic on CPU#0, kworker/u56:0/11 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-20230524-3.fc38 05/24/2023 Workqueue: cxl_port detach_memdev [cxl_core] RIP: 0010:spin_bug+0x65/0xa0 Call Trace: do_raw_spin_lock+0x69/0xa0 __mutex_lock+0x695/0xb80 delete_endpoint+0xad/0x150 [cxl_core] devres_release_all+0xb8/0x110 device_unbind_cleanup+0xe/0x70 device_release_driver_internal+0x1d2/0x210 detach_memdev+0x15/0x20 [cxl_core] process_one_work+0x1e3/0x4c0 worker_thread+0x1dd/0x3d0 2/ In the case of RCH topologies, the parent device that needs to be locked is not always @port->dev as returned by cxl_mem_find_port(), use endpoint->dev.parent instead. Fixes: 8dd2bc0 ("cxl/mem: Add the cxl_mem driver") Cc: <stable@vger.kernel.org> Reported-by: Robert Richter <rrichter@amd.com> Closes: http://lore.kernel.org/r/20231018171713.1883517-2-rrichter@amd.com Signed-off-by: Dan Williams <dan.j.williams@intel.com>
1 parent 8a749fd commit 8d2ad99

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

drivers/cxl/core/port.c

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,35 +1217,39 @@ static struct device *grandparent(struct device *dev)
12171217
return NULL;
12181218
}
12191219

1220+
static struct device *endpoint_host(struct cxl_port *endpoint)
1221+
{
1222+
struct cxl_port *port = to_cxl_port(endpoint->dev.parent);
1223+
1224+
if (is_cxl_root(port))
1225+
return port->uport_dev;
1226+
return &port->dev;
1227+
}
1228+
12201229
static void delete_endpoint(void *data)
12211230
{
12221231
struct cxl_memdev *cxlmd = data;
12231232
struct cxl_port *endpoint = cxlmd->endpoint;
1224-
struct cxl_port *parent_port;
1225-
struct device *parent;
1226-
1227-
parent_port = cxl_mem_find_port(cxlmd, NULL);
1228-
if (!parent_port)
1229-
goto out;
1230-
parent = &parent_port->dev;
1233+
struct device *host = endpoint_host(endpoint);
12311234

1232-
device_lock(parent);
1233-
if (parent->driver && !endpoint->dead) {
1234-
devm_release_action(parent, cxl_unlink_parent_dport, endpoint);
1235-
devm_release_action(parent, cxl_unlink_uport, endpoint);
1236-
devm_release_action(parent, unregister_port, endpoint);
1235+
device_lock(host);
1236+
if (host->driver && !endpoint->dead) {
1237+
devm_release_action(host, cxl_unlink_parent_dport, endpoint);
1238+
devm_release_action(host, cxl_unlink_uport, endpoint);
1239+
devm_release_action(host, unregister_port, endpoint);
12371240
}
12381241
cxlmd->endpoint = NULL;
1239-
device_unlock(parent);
1240-
put_device(parent);
1241-
out:
1242+
device_unlock(host);
12421243
put_device(&endpoint->dev);
1244+
put_device(host);
12431245
}
12441246

12451247
int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint)
12461248
{
1249+
struct device *host = endpoint_host(endpoint);
12471250
struct device *dev = &cxlmd->dev;
12481251

1252+
get_device(host);
12491253
get_device(&endpoint->dev);
12501254
cxlmd->endpoint = endpoint;
12511255
cxlmd->depth = endpoint->depth;

0 commit comments

Comments
 (0)