Re: [PATCH v3 4/5] cxl/region: Start unmapped poison scan at the committed decoder boundary

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing Out-of-Bounds array read in `poison_by_decoder()` when a decoder fails to map to a partition.
--

--- Patch [4]: [PATCH v3 4/5] cxl/region: Start unmapped poison scan at the committed decoder boundary ---
commit 3fc5ee5ec0b9d8fae7576a61bcdfd58c1e6ca1c5
Author: Richard Cheng <icheng@nvidia.com>

cxl/region: Start unmapped poison scan at the committed decoder boundary

poison_by_decoder() stops at the last committed decoder and records the
handoff in ctx->offset, but cxl_get_poison_unmapped() ignores it and
starts after the highest DPA allocation instead. Allocation exist for
uncommitted decoders too, so their DPA is skipped by both phases and
poison there is never reported. Resume the scan at ctx->offset, and scan
later partitions in full, restoring the pre-rewrite behavior.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 52ba8e9e42885..ba77416055f4a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2923,10 +2922,8 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  	 */
>  	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
>  		res = &cxlds->part[i].res;
> -		for (p = res->child, last = NULL; p; p = p->sibling)
> -			last = p;
> -		if (last)
> -			offset = last->end + 1;
> +		if (i == ctx->part)
> +			offset = ctx->offset;
>  		else
>  			offset = res->start;
>  		length = res->end - offset + 1;

[ ... ]

>  static int poison_by_decoder(struct device *dev, void *arg)
>  {
>  	struct cxl_poison_context *ctx = arg;
>  	struct cxl_endpoint_decoder *cxled;
>  	enum cxl_partition_mode mode;
>  	struct cxl_dev_state *cxlds;
>  	struct cxl_memdev *cxlmd;
>  	u64 offset, length;
>  	int rc = 0;
>  
>  	if (!is_endpoint_decoder(dev))
>  		return rc;
>  
>  	cxled = to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
>  		return rc;
>  
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;
>  	mode = cxlds->part[cxled->part].mode;

[Severity: High]
This is a pre-existing issue, but is it possible for cxled->part to be -1
here, causing an out-of-bounds read?

If a CXL decoder's DPA allocation is malformed or spans across partition
boundaries during initial enumeration, __cxl_dpa_reserve() can leave
cxled->part as -1 while still setting cxled->dpa_res.

Since this function only checks if cxled->dpa_res is non-NULL before the
array access, could a privileged user initiating a poison scan trigger a
read before the cxlds->part array? Other areas in the subsystem appear to
check if cxled->part < 0 prior to access.

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