Re: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder()

On 7/22/26 2:10 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The early return for decoders with `part < 0` skips the loop termination condition, causing iteration to silently continue past the last committed decoder.
> --
> 
> --- Patch [1]: [PATCH 1/3] cxl/region: Guard against unset partition in poison_by_decoder() ---
> commit 8424c8c335042ef0b39d03b8e79434f7d0ee9eba
> Author: Dave Jiang <[email protected]>
> 
> cxl/region: Guard against unset partition in poison_by_decoder()
> 
> This commit skips decoders with an unset partition (part < 0) in
> poison_by_decoder() to prevent an out-of-bounds access when querying
> partition modes.
> 
> Closes: https://sashiko.dev/#/patchset/[email protected]?part=4
> 
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 1e211542b6b64..ec0dc4284c3c5 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -2959,6 +2959,9 @@ static int poison_by_decoder(struct device *dev, void *arg)
>>  	if (!cxled->dpa_res)
>>  		return rc;
>>  
>> +	if (cxled->part < 0)
>> +		return rc;
> 
> [Severity: Medium]
> Does this early return bypass the iteration termination logic if the
> commit_end decoder itself has an unset partition?
> 
> Looking at poison_by_decoder(), returning early here skips the termination
> check at the bottom of the function:
> 
> 	/* Iterate until commit_end is reached */
> 	if (cxled->cxld.id == ctx->port->commit_end) {
> 		ctx->offset = cxled->dpa_res->end + 1;
> 		ctx->part = cxled->part;
> 		return 1;
> 	}
> 
> This would cause device_for_each_child() to continue iterating into 
> uncommitted decoders instead of returning 1 and stopping. Because the 
> iteration would ultimately return 0 instead of 1, cxl_get_poison_by_endpoint() 
> would then fail to trigger the subsequent cxl_get_poison_unmapped() call.

False positive. The intent is to iterate through all the child devices to find the commit end. Having cxled->part < 0 does not mean it is the commit_end. Iterating through uncommitted decoders just return 0 and is harmless. And if there's none found, then it doesn't matter if cxl_get_poison_by_endpoint() needs to be triggered.

DJ
> 
>> +
>>  	cxlmd = cxled_to_memdev(cxled);
>>  	cxlds = cxlmd->cxlds;
>>  	mode = cxlds->part[cxled->part].mode;
>