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

[Alison Schofield]

On Wed, May 20, 2026 at 07:59:21AM -0700, Dave Jiang wrote:
> 
> 
> On 5/20/26 5:30 AM, Li Ming wrote:
> > 
> > 在 2026/5/20 01:18, Dave Jiang 写道:
> >>
> >> On 5/19/26 6:23 AM, 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 -ENXIO in
> >> It can be set to other error codes I think? I would just s/-ENXIO/negative errno/
> > Sure, Will do that.
> >>
> >>> 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.
> >> Does the comment in cxl_region_sort_targets() need to be updated with the new changes?
> > 
> > I'm not sure how to update the comment in cxl_region_sort_targets(). Any suggestion?
> 
> idk if we should just drop it entirely since the comment is no longer true. At least that second part. Alison?

I'd like to see it replaced w this so we continue to have the
debug info, but stop the lie that led to this issue.

/*
 * Record that sorting failed, but still continue to calc
 * cxled->pos so that cxl_calc_interleave_pos() emits its
 * dev_dbg() for every member, which is useful for auto
 * discovery debug.
 */

snip

> >>> +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->nr_targets; i++) {
> >>> +        if (p->targets[i] == cxled) {
> >>> +            p->nr_targets--;
> >>> +            cxled->state = CXL_DECODER_STATE_AUTO;
> >>> +            cxled->pos = -1;
> >>> +            p->targets[i] = NULL;
> >>> +
> >>> +            return 1;
> >>> +        }
> >>> +    }

Sashiko review looks like it is calling out a valid 'hole' issue above.
Does the array need to be compacted when we remove an entry that is not
the last. That would keep nr_targets same as 'first free slot', so 
there are no NULL holes. I think that fix goes in a separate patch.