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

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

Pre-existing issues:
- [High] Hardware-provided interleave ways can exceed switch decoder target count, causing out-of-bounds heap writes.
--

--- Patch [1]: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path ---
commit bd336b4d1e6d9374287b7dbe87f0953733e4d653
Author: Alison Schofield <[email protected]>

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

The error path releases its reference to a switch decoder before logging an
error that includes the decoder name. If the released reference is the last
one, the decoder can be freed before the error message accesses its name.
Drop the reference after the error is reported.

> 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,
>  			break;
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but I have a question about the loop that
concludes here.

In drivers/cxl/core/region.c:find_pos_and_ways(), the code iterates up to the
interleave_ways read from the switch decoder:

	*ways = cxlsd->cxld.interleave_ways;

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

Is it possible for the hardware-provided interleave_ways to exceed the switch
decoder's actual target count (cxlsd->nr_targets)?

If the hardware control register specifies an interleave_ways value that is
larger than the capability used to size the flexible array, could this loop
read out-of-bounds from the cxlsd->target[] array?

Similarly, in drivers/cxl/core/port.c:decoder_populate_targets(), could this
mismatch also result in an out-of-bounds write when populating targets from
hardware registers without bounds checking against nr_targets?

> -	put_device(dev);
> -
>  	if (rc)
>  		dev_err(port->uport_dev,
>  			"failed to find %s:%s in target list of %s\n",
> @@ -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/3a36953b5c665cb9544ba61dafbea0b0fa508c77.1784654628.git.alison.schofield@intel.com?part=1