Re: [PATCH v2 2/4] cxl/region: Add region reference in memdev attach

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl/region: UAF due to dangling attach->cxlr pointer after endpoint teardown
--

commit c1388a867928fb269ed84b5e755417256bbf4bd5
Author: Alejandro Lucero <alucerop@amd.com>

cxl/region: Add region reference in memdev attach

This commit adds a new field in the cxl_attach_region struct to link it with
the region the memdev is attached to. This facilitates device links creation
where such a region is the supplier with non-PF0 physical functions wanting to
use the CXL region being the consumers.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c8..78ca7ebc3e557 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -4132,6 +4132,7 @@ int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
>  	if (rc)
>  		return rc;
>  
> +	attach->cxlr = cxlr;

[Severity: High]
Could this introduce a regression by leaving a dangling pointer that leads to a
use-after-free?

When the CXL port topology (endpoint) is unbound or torn down,
endpoint_unregister_region() is triggered via devm on the endpoint device.
This drops the reference to the region and potentially frees it.

However, the attach struct (bound to the parent PCIe PF0 device) outlives the
endpoint and retains this dangling attach->cxlr pointer, along with the
non-zero hpa_range.

If a secondary PF driver probes later, could it check the unmodified hpa_range
and dereference attach->cxlr->dev when creating a device link? Should there be
a corresponding cleanup action registered to clear this pointer?

>  	attach->hpa_range = (struct range) {
>  		.start = cxlr->params.res->start,
>  		.end = cxlr->params.res->end,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001132023.17032-1-alucerop@amd.com?part=2