Re: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked `interleave_ways` value from untrusted CXL device leads to heap out-of-bounds read and write on the `cxlsd->target[]` flexible array.
- [High] Out-of-bounds read of stack variable `target_list` when initializing HDM switch decoder targets.
--

--- Patch [1]: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path ---
commit 1c57e04f0aa13be6ae927836e151c3bc815eda70
Author: Alison Schofield <alison.schofield@intel.com>

cxl/region: Fix use-after-free in find_pos_and_ways() error path

This commit drops the reference to a switch decoder after logging an
error message that includes the decoder's name. This prevents a potential
use-after-free if it was the last reference.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b64..c86dd9d55e939 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1938,8 +1938,6 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,

[Severity: High]
This is a pre-existing issue, but is there a risk of an out-of-bounds array
access in this function and others when handling interleave_ways?

Just above this hunk, find_pos_and_ways() loops up to
cxlsd->cxld.interleave_ways and accesses the cxlsd->target[] flexible array:

	for (int i = 0; i < *ways; i++) {
		if (cxlsd->target[i] == port->parent_dport) {

The interleave_ways value originates from the HDM Decoder Control register in
init_hdm_decoder():

drivers/cxl/core/hdm.c:init_hdm_decoder() {
...
	rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl), &cxld->interleave_ways);
...
}

If an untrusted CXL device provides a small target_count but a maliciously
inflated interleave_ways, could this result in heap out-of-bounds reads here
in find_pos_and_ways(), as well as out-of-bounds writes in
decoder_populate_targets() and update_decoder_targets() when they iterate
up to the inflated value?

>  			break;
>  		}
>  	}
> -	put_device(dev);
> -
>  	if (rc)
>  		dev_err(port->uport_dev,
>  			"failed to find %s:%s in target list of %s\n",

[Severity: High]
This is also a pre-existing issue, but does this same unchecked
interleave_ways value cause a stack out-of-bounds read in
init_hdm_decoder()?

drivers/cxl/core/hdm.c:init_hdm_decoder() {
...
	for (i = 0; i < cxld->interleave_ways; i++)
		cxld->target_map[i] = target_list.target_id[i];
...
}

Since target_list.target_id is an 8-byte stack-allocated union, if
interleave_ways is larger than 8 (e.g., 16), could this loop read past
the end of the 8-byte array and leak adjacent stack memory into
cxld->target_map?

> @@ -1947,6 +1945,8 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  			dev_name(port->parent_dport->dport_dev),
>  			dev_name(&cxlsd->cxld.dev));
>  
> +	put_device(dev);
> +
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711180755.1779002-1-alison.schofield@intel.com?part=1