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

KobaK posted 1 patch 2 months ago
drivers/cxl/core/region.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH v2] cxl/region: Validate partition index before array access
Posted by KobaK 2 months ago
From: Koba Ko <kobak@nvidia.com>

construct_region() reads cxled->part and uses it to index
cxlds->part[] without checking for a negative value. If the
partition was never resolved, part remains at its initial value
of -1, causing an out-of-bounds array access.

Add a guard to return -EBUSY when part is negative.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Signed-off-by: Koba Ko <kobak@nvidia.com>
---
 drivers/cxl/core/region.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index edc267c6cf77..de749b54fd62 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3712,6 +3712,9 @@ 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)
+		return ERR_PTR(-EBUSY);
+
 	do {
 		cxlr = __create_region(cxlrd, cxlds->part[part].mode,
 				       atomic_read(&cxlrd->region_id),
-- 
2.43.0
Re: [PATCH v2] cxl/region: Validate partition index before array access
Posted by Dave Jiang 3 weeks, 3 days ago

On 4/13/26 7:45 PM, KobaK wrote:
> From: Koba Ko <kobak@nvidia.com>
> 
> construct_region() reads cxled->part and uses it to index
> cxlds->part[] without checking for a negative value. If the
> partition was never resolved, part remains at its initial value
> of -1, causing an out-of-bounds array access.
> 
> Add a guard to return -EBUSY when part is negative.
> 
> Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
> Signed-off-by: Koba Ko <kobak@nvidia.com>

Applied to cxl/next
abb3c0de1190

> ---
>  drivers/cxl/core/region.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index edc267c6cf77..de749b54fd62 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3712,6 +3712,9 @@ 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)
> +		return ERR_PTR(-EBUSY);
> +
>  	do {
>  		cxlr = __create_region(cxlrd, cxlds->part[part].mode,
>  				       atomic_read(&cxlrd->region_id),
Re: [PATCH v2] cxl/region: Validate partition index before array access
Posted by Alison Schofield 2 months ago
On Tue, Apr 14, 2026 at 10:45:27AM +0800, KobaK wrote:
> From: Koba Ko <kobak@nvidia.com>
> 
> construct_region() reads cxled->part and uses it to index
> cxlds->part[] without checking for a negative value. If the
> partition was never resolved, part remains at its initial value
> of -1, causing an out-of-bounds array access.
> 
> Add a guard to return -EBUSY when part is negative.
> 
> Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")

The above tag added the check for part < 0 in construct_region(),
so that's not the fixes tag. Like I wrote in v1, I don't see the
obvious tag that deleted that check, else I'd just tell you.

> Signed-off-by: Koba Ko <kobak@nvidia.com>
> ---

Need a changelog here.
See https://docs.kernel.org/process/submitting-patches.html#commentary


>  drivers/cxl/core/region.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index edc267c6cf77..de749b54fd62 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3712,6 +3712,9 @@ 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)
> +		return ERR_PTR(-EBUSY);
> +
>  	do {
>  		cxlr = __create_region(cxlrd, cxlds->part[part].mode,
>  				       atomic_read(&cxlrd->region_id),
> -- 
> 2.43.0
> 
>
Re: [PATCH v2] cxl/region: Validate partition index before array access
Posted by KobaK 2 months ago
On Mon, 13 Apr 2026 21:37:33 -0700, Alison Schofield wrote:
> > Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
>
> The above tag added the check for part < 0 in construct_region(),
> so that's not the fixes tag. Like I wrote in v1, I don't see the
> obvious tag that deleted that check, else I'd just tell you.

The check was lost during a merge resolution, not by any single
non-merge commit. How about dropping the Fixes tag entirely and
explaining in the body instead?

> > Signed-off-by: Koba Ko <kobak@nvidia.com>
> > ---
>
> Need a changelog here.
> See https://docs.kernel.org/process/submitting-patches.html#commentary

Sorry, missed it. Will add in v3.

- Koba
Re: [PATCH v2] cxl/region: Validate partition index before array access
Posted by Alison Schofield 2 months ago
On Tue, Apr 14, 2026 at 11:14:10PM +0800, KobaK wrote:
> On Mon, 13 Apr 2026 21:37:33 -0700, Alison Schofield wrote:
> > > Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
> >
> > The above tag added the check for part < 0 in construct_region(),
> > so that's not the fixes tag. Like I wrote in v1, I don't see the
> > obvious tag that deleted that check, else I'd just tell you.
> 
> The check was lost during a merge resolution, not by any single
> non-merge commit. How about dropping the Fixes tag entirely and
> explaining in the body instead?

Appreciate your follow-up Koba but a v3 is needless churn at
this point. DaveJ can fix up or drop the tag upon merging.

Note to DaveJ: see the v1 thread.

> 
> > > Signed-off-by: Koba Ko <kobak@nvidia.com>
> > > ---
> >
> > Need a changelog here.
> > See https://docs.kernel.org/process/submitting-patches.html#commentary
> 
> Sorry, missed it. Will add in v3.

No need to v3. Just a note for next time.

> 
> - Koba
Re: [PATCH v2] cxl/region: Validate partition index before array access
Posted by Alison Schofield 1 month ago
On Tue, Apr 14, 2026 at 10:05:06AM -0700, Alison Schofield wrote:
> On Tue, Apr 14, 2026 at 11:14:10PM +0800, KobaK wrote:
> > On Mon, 13 Apr 2026 21:37:33 -0700, Alison Schofield wrote:
> > > > Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
> > >
> > > The above tag added the check for part < 0 in construct_region(),
> > > so that's not the fixes tag. Like I wrote in v1, I don't see the
> > > obvious tag that deleted that check, else I'd just tell you.
> > 
> > The check was lost during a merge resolution, not by any single
> > non-merge commit. How about dropping the Fixes tag entirely and
> > explaining in the body instead?
> 
> Appreciate your follow-up Koba but a v3 is needless churn at
> this point. DaveJ can fix up or drop the tag upon merging.
> 
> Note to DaveJ: see the v1 thread.
> 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> > 
> > > > Signed-off-by: Koba Ko <kobak@nvidia.com>
> > > > ---
> > >
> > > Need a changelog here.
> > > See https://docs.kernel.org/process/submitting-patches.html#commentary
> > 
> > Sorry, missed it. Will add in v3.
> 
> No need to v3. Just a note for next time.
> 
> > 
> > - Koba
>