Re: [RFC PATCH 2/5] cxl/region: Factor decoder poison-list retrieval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/region: Out-of-bounds read on cxlds->part when cxled->part is -1
--

commit 15ab3ed0729f2bd36a81eed918520197e4a18f3a
Author: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>

cxl/region: Factor decoder poison-list retrieval

poison_by_decoder() performs two different operations: it queries an
unmapped DPA gap preceding an endpoint decoder, then queries the DPA
resource assigned to that decoder and its region.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 75b8092e6dc2f..c72524923b380 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2940,6 +2940,40 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
>  	return rc;
>  }
>  
> +enum cxl_poison_query_policy {
> +	CXL_POISON_QUERY_DIAGNOSTIC,
> +	CXL_POISON_QUERY_PREONLINE,
> +};
> +
> +static int cxl_get_poison_by_decoder(struct cxl_endpoint_decoder *cxled,
> +				     struct cxl_poison_snapshot *snapshot,
> +				     enum cxl_poison_query_policy policy)
> +{
> +	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	enum cxl_partition_mode mode;
> +	u64 offset, length;
> +	int rc;
> +
> +	if (!cxled->dpa_res)
> +		return 0;
> +
> +	mode = cxlds->part[cxled->part].mode;

[Severity: Medium]
Does this code read out of bounds when cxled->part is -1?

If a CXL device's hardware decoder configuration reserves a DPA range
that does not map to any valid partition, __cxl_dpa_reserve() gracefully
warns but returns success, leaving cxled->part = -1:

drivers/cxl/core/hdm.c:__cxl_dpa_reserve() {
	if (cxled->part < 0)
		for (int i = 0; i < cxlds->nr_partitions; i++)
			if (resource_contains(&cxlds->part[i].res, res)) {
				cxled->part = i;
				break;
			}

	if (cxled->part < 0)
		dev_warn(dev, "decoder%d.%d: %pr does not map any partition\n",
			 port->id, cxled->cxld.id, res);

	port->hdm_end++;
	get_device(&cxled->cxld.dev);
	return 0;
}

Later, when iterating over decoders, it skips the !cxled->dpa_res check and
unconditionally executes mode = cxlds->part[cxled->part].mode.

Since part array is embedded inside struct cxl_dev_state right after
dpa_res, part[-1] reads the tail padding or end of dpa_res within the
same memory allocation. This uses garbage data for the mode variable,
which can improperly mask -EFAULT errors if it randomly matches
CXL_PARTMODE_RAM.

Note that the pre-existing caller logic in poison_by_decoder() also retains
this out-of-bounds array access before invoking this new helper.

> +	offset = cxled->dpa_res->start;
> +	length = resource_size(cxled->dpa_res);
> +	if (snapshot)
> +		rc = cxl_mem_get_poison_snapshot(cxlmd, offset, length,
> +						 cxled->cxld.region, snapshot);
> +	else
> +		rc = cxl_mem_get_poison(cxlmd, offset, length,
> +					cxled->cxld.region);
> +
> +	if (rc == -EFAULT && mode == CXL_PARTMODE_RAM &&
> +	    policy == CXL_POISON_QUERY_DIAGNOSTIC)
> +		rc = 0;
> +	return rc;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010162017.62506-1-shaikhkamal2012@gmail.com?part=2