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

[Guixin Liu <kanie@linux.alibaba.com>]

在 2026/8/12 00:03, Li Ming 写道:
>
> 在 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 <kanie@linux.alibaba.com>
>> ---
>>   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.
Sure, changed in v2, thanks.

Best Regards,
Guixin Liu
>
>
>>   }
>>     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);