Re: [PATCH 3/8] cxl/core: Fix dport use-after-free via the einj_inject debugfs file

在 2026/8/11 19:36, Guixin Liu 写道:
> cxl_debugfs_create_dport_dir() creates a debugfs directory holding an
> "einj_inject" file whose i_private is the 'struct cxl_dport', but it
> discards the returned dentry and registers no cleanup. The dport is freed
> by free_dport() when the devres group of its host device is released,
> while the debugfs nodes live until the cxl_core module is unloaded
> (debugfs_remove_recursive() in cxl_core_exit()).
>
> So after unbinding the dport's host, e.g. unbinding the host bridge port
> or the ACPI0017 root, writing to
>
>    /sys/kernel/debug/cxl/<dport_dev>/einj_inject
>
> calls cxl_einj_inject() on freed memory and dereferences dport->rch and
> dport->dport_dev.
>
> The leaked directory is also named after the dport device, so re-adding the
> same dport (bind after unbind) hits an existing name, debugfs creation
> fails, and error injection stays broken for that dport for the rest of the
> module's lifetime.
>
> Save the dentry and drop the whole directory via a devm action on the same
> host device. The action is registered inside the dport devres group and
> after free_dport(), so it runs before the dport is freed. Propagate the
> failure to __devm_cxl_add_dport() rather than continuing with a dport that
> has a dangling debugfs node.
>
> Fixes: 8039804cfa73 ("cxl/core: Add CXL EINJ debugfs files")
> Signed-off-by: Guixin Liu <[email protected]>
> ---
>   drivers/cxl/core/port.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 1215ee4f4035..76bda54ed986 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -813,13 +813,18 @@ 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 cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
> +static void remove_debugfs(void *dentry)
> +{
> +	debugfs_remove_recursive(dentry);
> +}
> +
> +static int cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>   {
>   	struct cxl_port *parent = parent_port_of(dport->port);
>   	struct dentry *dir;
>   
>   	if (!einj_cxl_is_initialized())
> -		return;
> +		return 0;
>   
>   	/*
>   	 * Protocol error injection is only available for CXL 2.0+ root ports
> @@ -827,12 +832,15 @@ static void cxl_debugfs_create_dport_dir(struct cxl_dport *dport)
>   	 */
>   	if (!dport->rch &&
>   	    !(dev_is_pci(dport->dport_dev) && parent && is_cxl_root(parent)))
> -		return;
> +		return 0;
>   
>   	dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev));
>   
>   	debugfs_create_file("einj_inject", 0200, dir, dport,
>   			    &cxl_einj_inject_fops);
> +
> +	return devm_add_action_or_reset(dport_to_host(dport), remove_debugfs,
> +					dir);

I think we don't need to worry about devm_add_action_or_reset() failing, 
missing this debugfs directory seems like acceptable, but the failure 
cases of this devm_add_action_or_reset() will cause dport addition failure.

So I think just like below devm_cxl_dport_ras_setup(), do not check the 
return value of the function.

>   }
>   
>   static int cxl_port_add(struct cxl_port *port,
> @@ -1240,7 +1248,9 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>   	if (dev_is_pci(dport_dev))
>   		dport->link_latency = cxl_pci_get_latency(to_pci_dev(dport_dev));
>   
> -	cxl_debugfs_create_dport_dir(dport);
> +	rc = cxl_debugfs_create_dport_dir(dport);
> +	if (rc)
> +		return ERR_PTR(rc);
>   
>   	if (!dport->rch)
>   		devm_cxl_dport_ras_setup(dport);