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

[Dave Jiang]

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?


> 
> 
> Ming
> 
>>
>>> Fixes: 87805c32e6ad ("cxl/region: Fix use-after-free from auto assembly failure")
>>> Signed-off-by: Li Ming <ming.li@xxxxxxxxxxxx>
>> The rest LGTM
>>
>> DJ
>>
>>> ---
>>>   drivers/cxl/core/region.c | 35 ++++++++++++++++-------------------
>>>   1 file changed, 16 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>>> index e50dc716d4e8..551228bc91f5 100644
>>> --- a/drivers/cxl/core/region.c
>>> +++ b/drivers/cxl/core/region.c
>>> @@ -2202,18 +2202,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->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;
>>> +        }
>>> +    }
>>> +
>>> +    return 0;
>>>   }
>>>     /*
>>> @@ -2222,25 +2234,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 *
>>>
>>> ---
>>> base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
>>> change-id: 20260519-fix_out_of_bounds_access-8838759ee5a6
>>>
>>> Best regards,