Re: [PATCH v2] cxl/core: Fix dport use-after-free via the einj_inject debugfs file
Richard Cheng <[email protected]>
| Newsgroups | org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <anwlgJl8I_XhJ6dX@MWDK4CY14F> |
On Wed, Aug 12, 2026 at 02:09:43PM +0800, Guixin Liu wrote:
> 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
>
>
LGTM.
Reviewed-by: Richard Cheng <[email protected]>
Best regards,
Richard Cheng.