public inbox for linux-cxl@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cxl/region: Validate partition index before array access
@ 2026-04-09 15:44 KobaK
  2026-04-09 16:58 ` Dave Jiang
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: KobaK @ 2026-04-09 15:44 UTC (permalink / raw)
  To: Dave Jiang, Dan Williams
  Cc: Davidlohr Bueso, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ira Weiny, Li Ming, linux-cxl, linux-kernel, Koba Ko

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] cxl/region: Validate partition index before array access
  2026-04-09 15:44 [PATCH] cxl/region: Validate partition index before array access KobaK
@ 2026-04-09 16:58 ` Dave Jiang
  2026-04-09 18:26 ` KobaK
  2026-04-11 23:16 ` Alison Schofield
  2 siblings, 0 replies; 4+ messages in thread
From: Dave Jiang @ 2026-04-09 16:58 UTC (permalink / raw)
  To: KobaK, Dan Williams
  Cc: Davidlohr Bueso, Jonathan Cameron, Alison Schofield, Vishal Verma,
	Ira Weiny, Li Ming, linux-cxl, linux-kernel



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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cxl/region: Validate partition index before array access
  2026-04-09 15:44 [PATCH] cxl/region: Validate partition index before array access KobaK
  2026-04-09 16:58 ` Dave Jiang
@ 2026-04-09 18:26 ` KobaK
  2026-04-11 23:16 ` Alison Schofield
  2 siblings, 0 replies; 4+ messages in thread
From: KobaK @ 2026-04-09 18:26 UTC (permalink / raw)
  To: Dave Jiang; +Cc: Dan Williams, linux-cxl, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cxl/region: Validate partition index before array access
  2026-04-09 15:44 [PATCH] cxl/region: Validate partition index before array access KobaK
  2026-04-09 16:58 ` Dave Jiang
  2026-04-09 18:26 ` KobaK
@ 2026-04-11 23:16 ` Alison Schofield
  2 siblings, 0 replies; 4+ messages in thread
From: Alison Schofield @ 2026-04-11 23:16 UTC (permalink / raw)
  To: KobaK
  Cc: Dave Jiang, Dan Williams, Davidlohr Bueso, Jonathan Cameron,
	Vishal Verma, Ira Weiny, Li Ming, linux-cxl, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-11 23:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 15:44 [PATCH] cxl/region: Validate partition index before array access KobaK
2026-04-09 16:58 ` Dave Jiang
2026-04-09 18:26 ` KobaK
2026-04-11 23:16 ` Alison Schofield

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox