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>
---
 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

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
>

Thanks for the review. Will address the feedback in v2.

- Koba

On Thu, 9 Apr 2026 09:58:44 -0700, Dave Jiang wrote:
> Was this issue encountered during testing or just by inspection (or AI
> analysis)? I'm just curious on how this condition is triggered and if a
> regression unit test needs to be added.

Caught by AI-assisted code analysis.

Thanks,
Koba

On 4/9/26 8:44 AM, 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>

Was this issue encountered during testing or just by inspection (or AI analysis)? I'm just curious on how this condition is triggered and if a regression unit test needs to be added.

DJ

> ---
>  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),