[PATCH v2] cxl/core: Fix dport use-after-free via the einj_inject debugfs file
Guixin Liu <[email protected]>
| Newsgroups | org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <[email protected]> |
cxl_debugfs_create_dport_dir() publishes a debugfs directory containing an
"einj_inject" file whose i_private is the 'struct cxl_dport', then discards
the returned dentry and registers nothing to remove it. The other per-dport
facility set up next to it in __devm_cxl_add_dport(),
devm_cxl_dport_ras_setup(), binds its resources to dport_to_host(dport) so
that they go away with the dport. The debugfs directory has no such owner:
it lives until cxl_core is unloaded and cxl_core_exit() tears down the
whole cxl/ tree.
The dport itself is freed much earlier. free_dport() is registered in the
dport's devres group, so the dport is freed when the host device is
unbound, which is an ordinary sysfs operation on the host bridge port or
the ACPI0017 root, not a module-teardown-only path. After that unbind the
einj_inject file is still there, and a write to it calls cxl_einj_inject()
on freed memory, reading dport->rch and dport->dport_dev and passing them
to the EINJ code.
Re-binding the topology does not recover either. The stale directory keeps
the dport device's name, so the second creation finds the name in use,
debugfs setup fails, and error injection is silently unavailable for that
dport for the remaining lifetime of the module.
Keep the dentry and remove the directory from a devm action on the dport's
host device. The action is registered after free_dport() within the same
devres group, so release ordering runs it before the dport is freed. Its
registration failure is deliberately not propagated, following
devm_cxl_dport_ras_setup(): a missing debugfs directory is not a
functional failure of the dport, and on that path
devm_add_action_or_reset() has already removed the directory itself, so
there is no dangling node left and nothing to gain from failing the dport
addition.
Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
Signed-off-by: Guixin Liu <[email protected]>
---
This was patch 3/8 of the "cxl: Assorted fixes" series [1]. Per review
feedback that series is not being reworked as a whole; the fixes are resent
individually instead. Patches 1, 2 and 7 of the series are dropped, as those
issues are already fixed in cxl/next.
v1->v2:
- do not propagate the devm_add_action_or_reset() failure out of
cxl_debugfs_create_dport_dir(); a missing debugfs directory must not fail
the dport addition (Li Ming)
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
the code change (Alison Schofield)
[1] https://lore.kernel.org/linux-cxl/[email protected]/
drivers/cxl/core/port.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 625e4aa427db..62a9c2038d1f 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -814,6 +814,11 @@ static int cxl_einj_inject(void *data, u64 type)
DEFINE_DEBUGFS_ATTRIBUTE(cxl_einj_inject_fops, NULL, cxl_einj_inject,
"0x%llx\n");
+static void remove_debugfs(void *dentry)
+{
+ debugfs_remove_recursive(dentry);
+}
+
static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
{
struct cxl_port *parent = parent_port_of(dport->port);
@@ -834,6 +839,8 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
debugfs_create_file("einj_inject", 0200, dir, dport,
&cxl_einj_inject_fops);
+
+ devm_add_action_or_reset(dport_to_host(dport), remove_debugfs, dir);
}
static int cxl_port_add(struct cxl_port *port,
base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
--
2.43.7