Re: [PATCH v8 3/4] 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:
- [High] cxl: integer underflow in cxl_get_poison_unmapped() from unmapped decoder offset
--

commit 8602a2ffbae3a910b43392bb444acb65b6b365e7
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 042356c71ed79..4869cdd6a3d48 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2944,10 +2943,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;

[Severity: High]
Does this code risk an integer underflow if offset is greater than res->end?

If poison_by_decoder() encounters a zero-sized commit_end decoder, it can
set ctx->part = -1 and ctx->offset = cxled->dpa_res->end + 1. If that
decoder was configured by firmware with a large skip value, ctx->offset can
be strictly greater than the end of the first partition (part[0].res.end).

Later, cxl_get_poison_by_endpoint() forces ctx.part = 0 before calling
cxl_get_poison_unmapped():

drivers/cxl/core/region.c:cxl_get_poison_by_endpoint() {
    ...
	if (rc == 1) {
		/* ... */
		if (ctx.part < 0)
			ctx.part = 0;
		rc = cxl_get_poison_unmapped(to_cxl_memdev(port->uport_dev),
					     &ctx);
	}
    ...
}

Because ctx.offset is not reset to part[0].res.start, offset is assigned
the out-of-bounds ctx->offset when i == 0. This causes the length
calculation to underflow in unsigned 64-bit arithmetic to an enormous
positive value.

Could this underflow bypass the !length check and result in sending an
invalid mailbox command to the hardware?

A massive out-of-bounds length passed directly to cxl_mem_get_poison() may
cause the CXL device to hang, time out, or process out-of-bounds requests,
leading to system instability.

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