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

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() erro=
r handling ---
commit b12d8e90580fa9f2baf0c0dcbd4c6c514e0b2343
Author: Richard Cheng <[email protected]>

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;
>  };
> =20
> +/*
> + * 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_mod=
e mode)
> +{
> +	return rc =3D=3D -EFAULT && mode =3D=3D 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=20
memdev.c that could leverage this new helper instead of leaving it open-cod=
ed.

[ ... ]

> @@ -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 =3D 0;
> +	int rc;
> =20
>  	if (!is_endpoint_decoder(dev))
> -		return rc;
> +		return 0;
> =20
>  	cxled =3D to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
> -		return rc;
> +		return 0;
> =20
>  	cxlmd =3D cxled_to_memdev(cxled);
>  	cxlds =3D 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 =3D cxlds->part[cxled->par=
t].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 >=3D 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.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260805055524.2231=
[email protected]?part=3D1