Re: [PATCH v5 6/7] cxl/region: Reject poison scan for decoder without a partition

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Returning -ENODEV from poison_by_decoder() for unmapped decoders aborts the entire endpoint's poison list collection.
--

--- Patch [6]: [PATCH v5 6/7] cxl/region: Reject poison scan for decoder without a partition ---
commit 8d92957ad7710c4351ca4406f4bb3f3474e28442
Author: Richard Cheng <[email protected]>

cxl/region: Reject poison scan for decoder without a partition

This commit aims to prevent an out-of-bounds array read in
poison_by_decoder() by returning early when a decoder has no assigned
partition. It checks if the partition is negative and returns -ENODEV
before indexing the partition array.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b7dc5d4988da7..afe3fb57b7fe2 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2954,6 +2954,8 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  	cxled = to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
>  		return rc;
> +	if (cxled->part < 0)
> +		return -ENODEV;

[Severity: Medium]
Will returning -ENODEV here abort the iteration for the entire endpoint?

Since poison_by_decoder() is used as a callback, returning a non-zero error
code causes device_for_each_child() to stop iterating immediately. This would
completely prevent the retrieval of poison data for all other valid, mapped
decoders on the same device.

Would it be better to return 0 instead, which would safely skip the unmapped
decoder but allow the poison scan to continue for the remaining decoders?

>  
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6