Skip to content

Commit 19d2f0b

Browse files
AlisonSchofielddavejiang
authored andcommitted
cxl/port: Fix use after free of parent_port in cxl_detach_ep()
cxl_detach_ep() is called during bottom-up removal when all CXL memory devices beneath a switch port have been removed. For each port in the hierarchy it locks both the port and its parent, removes the endpoint, and if the port is now empty, marks it dead and unregisters the port by calling delete_switch_port(). There are two places during this work where the parent_port may be used after freeing: First, a concurrent detach may have already processed a port by the time a second worker finds it via bus_find_device(). Without pinning parent_port, it may already be freed when we discover port->dead and attempt to unlock the parent_port. In a production kernel that's a silent memory corruption, with lock debug, it looks like this: []DEBUG_LOCKS_WARN_ON(__owner_task(owner) != get_current()) []WARNING: kernel/locking/mutex.c:949 at __mutex_unlock_slowpath+0x1ee/0x310 []Call Trace: []mutex_unlock+0xd/0x20 []cxl_detach_ep+0x180/0x400 [cxl_core] []devm_action_release+0x10/0x20 []devres_release_all+0xa8/0xe0 []device_unbind_cleanup+0xd/0xa0 []really_probe+0x1a6/0x3e0 Second, delete_switch_port() releases three devm actions registered against parent_port. The last of those is unregister_port() and it calls device_unregister() on the child port, which can cascade. If parent_port is now also empty the device core may unregister and free it too. So by the time delete_switch_port() returns, parent_port may be free, and the subsequent device_unlock(&parent_port->dev) operates on freed memory. The kernel log looks same as above, with a different offset in cxl_detach_ep(). Both of these issues stem from the absence of a lifetime guarantee between a child port and its parent port. Establish a lifetime rule for ports: child ports hold a reference to their parent device until release. Take the reference when the port is allocated and drop it when released. This ensures the parent is valid for the full lifetime of the child and eliminates the use after free window in cxl_detach_ep(). This is easily reproduced with a reload of cxl_acpi in QEMU with CXL devices present. Fixes: 2345df5 ("cxl/memdev: Fix endpoint port removal") Reviewed-by: Dave Jiang <[email protected]> Reviewed-by: Li Ming <[email protected]> Signed-off-by: Alison Schofield <[email protected]> Reviewed-by: Jonathan Cameron <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Dave Jiang <[email protected]>
1 parent 11439c4 commit 19d2f0b

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

drivers/cxl/core/port.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -552,10 +552,13 @@ static void cxl_port_release(struct device *dev)
552552
xa_destroy(&port->dports);
553553
xa_destroy(&port->regions);
554554
ida_free(&cxl_port_ida, port->id);
555-
if (is_cxl_root(port))
555+
556+
if (is_cxl_root(port)) {
556557
kfree(to_cxl_root(port));
557-
else
558+
} else {
559+
put_device(dev->parent);
558560
kfree(port);
561+
}
559562
}
560563

561564
static ssize_t decoders_committed_show(struct device *dev,
@@ -707,6 +710,7 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev,
707710
struct cxl_port *iter;
708711

709712
dev->parent = &parent_port->dev;
713+
get_device(dev->parent);
710714
port->depth = parent_port->depth + 1;
711715
port->parent_dport = parent_dport;
712716

0 commit comments

Comments
 (0)