Re: [PATCH] cxl/region: Fix NULL pointer within p->targets[]

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] __cxl_decoder_detach() suffers from the exact same p->targets hole vulnerability for fully assembled auto-discovered regions, which will cause a NULL pointer dereference upon re-attachment.
- [High] Failed auto-discovery decoders are left in the region targets array if failure occurs after endpoint port attachment, leading to a Use-After-Free (UAF).
--

commit 8beb15c55def2c0ba7cda46a708f6b3d2ab5e643
Author: Li Ming <ming.li@zohomail.com>

cxl/region: Fix NULL pointer within p->targets[]

This patch fixes a NULL pointer issue in p->targets[] during endpoint
decoder removal. It ensures the last valid endpoint decoder pointer is
swapped into the removed slot, avoiding gaps in the valid targets array.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e90c024c80365..54018db87a4c9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2220,7 +2220,15 @@ static int cxl_region_remove_target(struct device *dev, void *data)
>  			p->nr_targets--;
>  			cxled->state = CXL_DECODER_STATE_AUTO;
>  			cxled->pos = -1;
> -			p->targets[i] = NULL;
> +
> +			/*
> +			 * Swap the last valid target into the slot to
> +			 * ensure no invalid target in p->nr_targets range.
> +			 * The targets array will be re-sorted during the
> +			 * last endpoint decoder attaching again.
> +			 */
> +			p->targets[i] = p->targets[p->nr_targets];
> +			p->targets[p->nr_targets] = NULL;

[Severity: High]
This isn't a bug introduced by this patch, but does __cxl_decoder_detach()
suffer from the exact same p->targets hole vulnerability for fully assembled
auto-discovered regions?

When a target is detached from an auto-discovered region:

drivers/cxl/core/region.c:__cxl_decoder_detach() {
    ...
	p->targets[cxled->pos] = NULL;
	p->nr_targets--;
    ...
}

For auto-discovered regions, new targets are appended at
p->targets[p->nr_targets] by cxl_region_attach_auto(). This leaves the NULL
hole intact while overwriting a valid endpoint pointer that was previously
positioned at the end of the array. Once p->nr_targets reaches
p->interleave_ways again, cxl_region_sort_targets() iterates through the
array and attempts to dereference the NULL pointer, which could cause a
kernel panic.


[Severity: High]
This isn't a bug introduced by this patch, but are failed auto-discovery
decoders left in the region targets array if failure occurs after endpoint
port attachment, leading to a Use-After-Free?

cxl_rr_ep_add() correctly upgrades an endpoint's staging state to
CXL_DECODER_STATE_AUTO:

drivers/cxl/core/region.c:cxl_rr_ep_add() {
    ...
		if (cxld == &cxled->cxld &&
		    cxled->state == CXL_DECODER_STATE_AUTO_STAGED)
			cxled->state = CXL_DECODER_STATE_AUTO;
    ...
}

However, if subsequent topology validation fails, the error path calls
cxl_port_detach_region(), which clears the region pointer but neglects to
restore the endpoint's state back to CXL_DECODER_STATE_AUTO_STAGED. Because
of this, when __cxl_decoder_detach() invokes cxl_cancel_auto_attach(), it
silently skips removal due to the state mismatch:

drivers/cxl/core/region.c:cxl_cancel_auto_attach() {
    ...
	if (cxled->state != CXL_DECODER_STATE_AUTO_STAGED)
		return;
    ...
}

The endpoint decoder remains in the p->targets array and is subsequently
freed upon removal, resulting in a persistent Use-After-Free vulnerability
when the sysfs region targets are read or the array is otherwise accessed.

>  
>  			return 1;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530-fix_null_in_targets_array-v1-1-312c3bf1fe0f@zohomail.com?part=1