Re: [PATCH v2 1/1] cxl/region: Fix out-of-bounds access in cxl_cancel_auto_attach()

[Alison Schofield]

On Sun, May 24, 2026 at 05:56:20PM +0800, Li Ming wrote:
> In cxl_cancel_auto_attach(), it assumes cxled->pos is a valid index for
> accessing p->targets[]. However, cxled->pos can be set to negative errno
> in cxl_region_sort_targets() if cxl_calc_interleave_pos() fails. This
> causes the driver to use a negative index to access p->targets[],
> resulting in out-of-bounds access.
> 
> Fix it by walking p->targets[] instead of using cxled->pos directly.
> 
> Fixes: 87805c32e6ad ("cxl/region: Fix use-after-free from auto assembly failure")
> Signed-off-by: Li Ming <ming.li@xxxxxxxxxxxx>
> ---
> Changes from v1:
> - Update commit log. (Dave)
> - Update comment in cxl_region_sort_targets(). (Dave)
> - Fix potential 'hole' issue. (Alison & Sashiko review)

The switch in v2 to walk using p->interleave_ways instead of
p->nr_targets fixes the 'hole' issue inside the cancel walker
but there are more places where a corrupt target array will
cause problems. 

cxl_region_attach_auto() and cxl_region_sort_targets() both assume
that p->targets[] has no NULL gap. We need to compact that array on
cancel so that nr_targets remains the index of the first free slot,
with no gap below it.

If you do want to fix both things in one patch, then uplevel the subject
line to something like:
        cxl/region: Fix corruption of p->targets[] on auto-attach cancellation
and have the body describe and address both the OOB access and the compaction.

BTW I don't feel too strongly about combined vs separate patches, but I
don't think this OOB patch should add in a partial fixup for the hole issue.

> ---
>  drivers/cxl/core/region.c | 40 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e50dc716d4e8..8956b049c462 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2011,8 +2011,9 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr)
>  		cxled->pos = cxl_calc_interleave_pos(cxled, &cxlr->hpa_range);
>  		/*
>  		 * Record that sorting failed, but still continue to calc
> -		 * cxled->pos so that follow-on code paths can reliably
> -		 * do p->targets[cxled->pos] to self-reference their entry.
> +		 * cxled->pos so that cxl_calc_interleave_pos() emits its
> +		 * dev_dbg() for every member. which is useful for auto
> +		 * discovery debug.
>  		 */
>  		if (cxled->pos < 0)
>  			rc = -ENXIO;
> @@ -2202,18 +2203,30 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	return 0;
>  }
>  
> -static int cxl_region_by_target(struct device *dev, const void *data)
> +static int cxl_region_remove_target(struct device *dev, void *data)
>  {
> -	const struct cxl_endpoint_decoder *cxled = data;
> +	struct cxl_endpoint_decoder *cxled = data;
>  	struct cxl_region_params *p;
>  	struct cxl_region *cxlr;
> +	int i;
>  
>  	if (!is_cxl_region(dev))
>  		return 0;
>  
>  	cxlr = to_cxl_region(dev);
>  	p = &cxlr->params;
> -	return p->targets[cxled->pos] == cxled;
> +	for (i = 0; i < p->interleave_ways; i++) {
> +		if (p->targets[i] == cxled) {
> +			p->nr_targets--;
> +			cxled->state = CXL_DECODER_STATE_AUTO;
> +			cxled->pos = -1;
> +			p->targets[i] = NULL;
> +
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -2222,25 +2235,10 @@ static int cxl_region_by_target(struct device *dev, const void *data)
>   */
>  static void cxl_cancel_auto_attach(struct cxl_endpoint_decoder *cxled)
>  {
> -	struct cxl_region_params *p;
> -	struct cxl_region *cxlr;
> -	int pos = cxled->pos;
> -
>  	if (cxled->state != CXL_DECODER_STATE_AUTO_STAGED)
>  		return;
>  
> -	struct device *dev __free(put_device) =
> -		bus_find_device(&cxl_bus_type, NULL, cxled, cxl_region_by_target);
> -	if (!dev)
> -		return;
> -
> -	cxlr = to_cxl_region(dev);
> -	p = &cxlr->params;
> -
> -	p->nr_targets--;
> -	cxled->state = CXL_DECODER_STATE_AUTO;
> -	cxled->pos = -1;
> -	p->targets[pos] = NULL;
> +	bus_for_each_dev(&cxl_bus_type, NULL, cxled, cxl_region_remove_target);
>  }
>  
>  static struct cxl_region *
> -- 
> 2.43.0
>