Re: [PATCH] cxl/region: Validate partition index before array access

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Apr 09, 2026 at 11:44:45PM +0800, KobaK wrote:
> From: Koba Ko <kobak@nvidia.com>
> 
> Check partition index bounds before accessing cxlds->part[] to prevent
> out-of-bounds access when part is -1 or invalid.
> 
> The partition index is read from cxled->part without validation. If it's
> negative or exceeds nr_partitions, accessing cxlds->part[part].mode will
> cause out-of-bounds array access.
> 
> Fixes: 5ec67596e368 ("cxl/region: Drop goto pattern of construct_region()")
> Signed-off-by: Koba Ko <kobak@nvidia.com>


This bounds check need is overstated. All writers of cxled->part either
initialize it to -1 or assign it from a bounded walk of cxlds->nr_partitions.
There is no path in that code that produces a positive out-of-range index,
and the only invalid state is part == -1. So, let's focus on the -1 case.

(If we did want to defensively guard against out-of-range indices, that
would need to be applied consistently across all accesses of cxlds->part[part],
not just in construct_region(). I don't think that is needed.)

The Fixes: Tag was a bit perplexing. The cited commit does touch any lines
related to partition indexing, at least based on "git show 5ec67596e368".
However, a close look suggests something else is going on.

Commit be5cbd084027 ("cxl: Kill enum cxl_decoder_mode") which previously
reworked this code included the needed check:

+	int rc, part = READ_ONCE(cxled->part);+
+       if (part < 0)
+               return ERR_PTR(-EBUSY);

And that check is indeed preserved in the final lore posting of the patch
referenced in the Fixes: tag:
https://lore.kernel.org/all/20250221013205.126419-1-ming.li@zohomail.com/

The version that landed in the tree however is different:
5ec67596e368 ("cxl/region: Drop goto pattern of construct_region()")

This suggests the check was not removed by that commit directly but instead
lost during a merge.

At this point, I suggest a v2 of 'this' patch that simply restores the
missing check, no added range check, no debug messaging, simply:

	+       if (part < 0)
	+               return ERR_PTR(-EBUSY);


It would be useful for Ming and DaveJ to take a close look at the merge
to confirm if anything else is off.


> ---
>  drivers/cxl/core/region.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index edc267c6cf77a..6be46636db7ee 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3712,6 +3712,14 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  	int rc, part = READ_ONCE(cxled->part);
>  	struct cxl_region *cxlr;
>  
> +	if (part < 0 || part >= cxlds->nr_partitions) {
> +		dev_err(cxlmd->dev.parent,
> +			"%s:%s: invalid partition index %d (max %u)\n",
> +			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> +			part, cxlds->nr_partitions);
> +		return ERR_PTR(-ENXIO);
> +	}
> +
>  	do {
>  		cxlr = __create_region(cxlrd, cxlds->part[part].mode,
>  				       atomic_read(&cxlrd->region_id),
> -- 
> 2.43.0
>