Re: [PATCH v21 01/23] cxl/mem: refactor memdev allocation

[Alejandro Lucero Palau <alucerop@amd.com>]

On 11/20/25 20:27, Koralahalli Channabasappa, Smita wrote:
> Hi Alejandro,
>

Hi,


<snip>


> On 11/19/2025 11:22 AM, alejandro.lucero-palau@amd.com wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> +
>> +static void __cxlmd_free(struct cxl_memdev *cxlmd)
>> +{
>> +    if (IS_ERR(cxlmd))
>> +        return;
>> +
>> +    if (cxlmd->cxlds)
>> +        cxlmd->cxlds->cxlmd = NULL;
>> +
>
> This series caused a NULL deref in devm_cxl_add_memdev().
> __cxlmd_free() only checks IS_ERR(cxlmd) and proceeds to dereference 
> cxlmd->cxlds.
>
> Adding a NULL check for cxlmd fixed the crash in my setup.
>

Yes. Believe it or not, but I 'm pretty sure I added that after the 
IS_ERR check, but it seems I spoiled it with the refactoring.


But thank you for reporting it. I'll fix it in v22.


Thank you


> BUG: kernel NULL pointer dereference, address: 0000000000000358
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 1553a7067 P4D 0
> Oops: Oops: 0000 [#1] SMP NOPTI
> RIP: 0010:devm_cxl_add_memdev+0x71/0xb0 [cxl_mem]
> Code: 89 c4 e8 c2 c8 be f8 85 c0 75 17 48 89 de 4c 89 ef e8 b3 08 f9 
> ff 85 c0 75 08 45 31 e4 45 31 ed eb 08 48 98 49 89 dd 48 89 c3 <49> 8b 
> 85 58 03 00 00 48 85 c0 74 08 48 c7 40 08 00 00 00 00 4c 89
> CR2: 0000000000000358 CR3: 00000001553a6002 CR4: 0000000000771ef0
> PKRU: 55555554
> Call Trace:
> <TASK>
> cxl_pci_probe+0x409/0xb00 [cxl_pci]
> ? update_load_avg+0x83/0x780
> local_pci_probe+0x4d/0xb0
> work_for_cpu_fn+0x1e/0x30
> process_scheduled_works+0xa9/0x420
> ? __pfx_worker_thread+0x10/0x10
> worker_thread+0x127/0x270
> ...
>
> Thanks
> Smita
>
>> +    put_device(&cxlmd->dev);
>> +    kfree(cxlmd);
>> +}
>> +
>> +DEFINE_FREE(cxlmd_free, struct cxl_memdev *, __cxlmd_free(_T))
>> +
>> +/**
>> + * devm_cxl_add_memdev - Add a CXL memory device
>> + * @host: devres alloc/release context and parent for the memdev
>> + * @cxlds: CXL device state to associate with the memdev
>> + *
>> + * Upon return the device will have had a chance to attach to the
>> + * cxl_mem driver, but may fail if the CXL topology is not ready
>> + * (hardware CXL link down, or software platform CXL root not attached)
>> + */
>> +struct cxl_memdev *devm_cxl_add_memdev(struct device *host,
>> +                       struct cxl_dev_state *cxlds)
>> +{
>> +    struct cxl_memdev *cxlmd __free(cxlmd_free) = 
>> cxl_memdev_alloc(cxlds);
>> +    int rc;
>> +
>> +    if (IS_ERR(cxlmd))
>> +        return cxlmd;
>> +
>> +    rc = dev_set_name(&cxlmd->dev, "mem%d", cxlmd->id);
>>       if (rc)
>> -        goto err;
>> +        return ERR_PTR(rc);
>>   -    rc = devm_add_action_or_reset(host, cxl_memdev_unregister, 
>> cxlmd);
>> +    rc = devm_cxl_memdev_add_or_reset(host, cxlmd);
>>       if (rc)
>>           return ERR_PTR(rc);
>> -    return cxlmd;
>>   -err:
>> -    /*
>> -     * The cdev was briefly live, shutdown any ioctl operations that
>> -     * saw that state.
>> -     */
>> -    cxl_memdev_shutdown(dev);
>> -    put_device(dev);
>> -    return ERR_PTR(rc);
>> +    return no_free_ptr(cxlmd);
>>   }
>>   EXPORT_SYMBOL_NS_GPL(devm_cxl_add_memdev, "CXL");
>>   diff --git a/drivers/cxl/private.h b/drivers/cxl/private.h
>> new file mode 100644
>> index 000000000000..50c2ac57afb5
>> --- /dev/null
>> +++ b/drivers/cxl/private.h
>> @@ -0,0 +1,10 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2025 Intel Corporation. */
>> +
>> +/* Private interfaces betwen common drivers ("cxl_mem") and the 
>> cxl_core */
>> +
>> +#ifndef __CXL_PRIVATE_H__
>> +#define __CXL_PRIVATE_H__
>> +struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds);
>> +int devm_cxl_memdev_add_or_reset(struct device *host, struct 
>> cxl_memdev *cxlmd);
>> +#endif /* __CXL_PRIVATE_H__ */
>