Re: [PATCH v9 1/3] cxl/region: Simplify poison_by_decoder() error handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The `poison_efault_forgiven()` helper is restricted to `region.c`, leaving identical open-coded logic in `memdev.c`.

Pre-existing issues:
- [High] Unchecked array index `cxled->part` can be `-1`, leading to an out-of-bounds access on `cxlds->part`.
--

--- Patch [1]: [PATCH v9 1/3] cxl/region: Simplify poison_by_decoder() error handling ---
commit b12d8e90580fa9f2baf0c0dcbd4c6c514e0b2343
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Simplify poison_by_decoder() error handling

This patch simplifies error handling in cxl_get_poison_unmapped() and
poison_by_decoder() by testing the -EFAULT condition directly and returning
a literal 0 to continue the walk. It introduces poison_efault_forgiven()
to consolidate this logic.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..45536909cc17d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2905,6 +2905,16 @@ struct cxl_poison_context {
>  	u64 offset;
>  };
>  
> +/*
> + * A device may answer a Get Poison List request with "physical address
> + * specified is invalid" (-EFAULT). That answer is tolerated for a RAM
> + * partition and the poison walk continues.
> + */
> +static inline bool poison_efault_forgiven(int rc, enum cxl_partition_mode mode)
> +{
> +	return rc == -EFAULT && mode == CXL_PARTMODE_RAM;
> +}

[Severity: Low]
Since this helper is defined locally here in region.c, could it be moved to a
shared internal header like drivers/cxl/cxlmem.h?

There is identical open-coded logic in cxl_get_poison_by_memdev() within 
memdev.c that could leverage this new helper instead of leaving it open-coded.

[ ... ]

> @@ -2950,14 +2960,14 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  	struct cxl_dev_state *cxlds;
>  	struct cxl_memdev *cxlmd;
>  	u64 offset, length;
> -	int rc = 0;
> +	int rc;
>  
>  	if (!is_endpoint_decoder(dev))
> -		return rc;
> +		return 0;
>  
>  	cxled = to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
> -		return rc;
> +		return 0;
>  
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;

[Severity: High]
This is a pre-existing issue, but does this code risk an out-of-bounds array
access?

Right after these lines, the code evaluates mode = cxlds->part[cxled->part].mode.

If cxl_dpa_alloc() allocates DPA capacity but cannot strictly map it to a
partition, it can leave cxled->part initialized to -1.

When sysfs invokes poison_by_decoder(), this evaluates cxlds->part[-1].mode
before ensuring cxled->part >= 0. This performs an out-of-bounds read into
memory preceding the array (such as the dpa_res struct) and could silently
corrupt the poison evaluation logic or trigger memory safety warnings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805055524.22311-1-icheng@nvidia.com?part=1