Linux CXL
 help / color / mirror / Atom feed
* [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path
@ 2026-07-11 18:07 Alison Schofield
  2026-07-11 18:19 ` sashiko-bot
  2026-07-13  2:48 ` Li Ming
  0 siblings, 2 replies; 4+ messages in thread
From: Alison Schofield @ 2026-07-11 18:07 UTC (permalink / raw)
  To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Ira Weiny, Dan Williams, Li Ming
  Cc: linux-cxl

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.

Found via inspection.

Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/region.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 1e211542b6b6..c86dd9d55e93 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;
 		}
 	}
-	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;
 }
 

base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.37.3


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

* Re: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path
  2026-07-11 18:07 [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path Alison Schofield
@ 2026-07-11 18:19 ` sashiko-bot
  2026-07-11 19:52   ` Alison Schofield
  2026-07-13  2:48 ` Li Ming
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-07-11 18:19 UTC (permalink / raw)
  To: Alison Schofield; +Cc: linux-cxl

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

Pre-existing issues:
- [High] Unchecked `interleave_ways` value from untrusted CXL device leads to heap out-of-bounds read and write on the `cxlsd->target[]` flexible array.
- [High] Out-of-bounds read of stack variable `target_list` when initializing HDM switch decoder targets.
--

--- Patch [1]: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path ---
commit 1c57e04f0aa13be6ae927836e151c3bc815eda70
Author: Alison Schofield <alison.schofield@intel.com>

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

This commit drops the reference to a switch decoder after logging an
error message that includes the decoder's name. This prevents a potential
use-after-free if it was the last reference.

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

[Severity: High]
This is a pre-existing issue, but is there a risk of an out-of-bounds array
access in this function and others when handling interleave_ways?

Just above this hunk, find_pos_and_ways() loops up to
cxlsd->cxld.interleave_ways and accesses the cxlsd->target[] flexible array:

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

The interleave_ways value originates from the HDM Decoder Control register in
init_hdm_decoder():

drivers/cxl/core/hdm.c:init_hdm_decoder() {
...
	rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl), &cxld->interleave_ways);
...
}

If an untrusted CXL device provides a small target_count but a maliciously
inflated interleave_ways, could this result in heap out-of-bounds reads here
in find_pos_and_ways(), as well as out-of-bounds writes in
decoder_populate_targets() and update_decoder_targets() when they iterate
up to the inflated value?

>  			break;
>  		}
>  	}
> -	put_device(dev);
> -
>  	if (rc)
>  		dev_err(port->uport_dev,
>  			"failed to find %s:%s in target list of %s\n",

[Severity: High]
This is also a pre-existing issue, but does this same unchecked
interleave_ways value cause a stack out-of-bounds read in
init_hdm_decoder()?

drivers/cxl/core/hdm.c:init_hdm_decoder() {
...
	for (i = 0; i < cxld->interleave_ways; i++)
		cxld->target_map[i] = target_list.target_id[i];
...
}

Since target_list.target_id is an 8-byte stack-allocated union, if
interleave_ways is larger than 8 (e.g., 16), could this loop read past
the end of the 8-byte array and leak adjacent stack memory into
cxld->target_map?

> @@ -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/20260711180755.1779002-1-alison.schofield@intel.com?part=1

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

* Re: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path
  2026-07-11 18:19 ` sashiko-bot
@ 2026-07-11 19:52   ` Alison Schofield
  0 siblings, 0 replies; 4+ messages in thread
From: Alison Schofield @ 2026-07-11 19:52 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-cxl

On Sat, Jul 11, 2026 at 06:19:49PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unchecked `interleave_ways` value from untrusted CXL device leads to heap out-of-bounds read and write on the `cxlsd->target[]` flexible array.
> - [High] Out-of-bounds read of stack variable `target_list` when initializing HDM switch decoder targets.

For both of these...
interleave_ways is not fully unchecked. It is decoded by eiw_to_ways(),
which rejects any encoding outside the legal set and caps the result at
16, and init_hdm_decoder() checks that return value and bails on error.
So the value is always in [1, 16]

But - as noted here it could exceed HW target list or reported target
count. That would happen w a bad device, ie malfunctioning or malicious
since good hardware won't advertise an interleave that exceeds its own
target list.

Worth hardening, and I'll follow w the fixup.


> --
> 
> --- Patch [1]: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path ---
> commit 1c57e04f0aa13be6ae927836e151c3bc815eda70
> Author: Alison Schofield <alison.schofield@intel.com>
> 
> cxl/region: Fix use-after-free in find_pos_and_ways() error path
> 
> This commit drops the reference to a switch decoder after logging an
> error message that includes the decoder's name. This prevents a potential
> use-after-free if it was the last reference.
> 
> > 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,
> 
> [Severity: High]
> This is a pre-existing issue, but is there a risk of an out-of-bounds array
> access in this function and others when handling interleave_ways?
> 
> Just above this hunk, find_pos_and_ways() loops up to
> cxlsd->cxld.interleave_ways and accesses the cxlsd->target[] flexible array:
> 
> 	for (int i = 0; i < *ways; i++) {
> 		if (cxlsd->target[i] == port->parent_dport) {
> 
> The interleave_ways value originates from the HDM Decoder Control register in
> init_hdm_decoder():
> 
> drivers/cxl/core/hdm.c:init_hdm_decoder() {
> ...
> 	rc = eiw_to_ways(FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl), &cxld->interleave_ways);
> ...
> }
> 
> If an untrusted CXL device provides a small target_count but a maliciously
> inflated interleave_ways, could this result in heap out-of-bounds reads here
> in find_pos_and_ways(), as well as out-of-bounds writes in
> decoder_populate_targets() and update_decoder_targets() when they iterate
> up to the inflated value?
> 
> >  			break;
> >  		}
> >  	}
> > -	put_device(dev);
> > -
> >  	if (rc)
> >  		dev_err(port->uport_dev,
> >  			"failed to find %s:%s in target list of %s\n",
> 
> [Severity: High]
> This is also a pre-existing issue, but does this same unchecked
> interleave_ways value cause a stack out-of-bounds read in
> init_hdm_decoder()?
> 
> drivers/cxl/core/hdm.c:init_hdm_decoder() {
> ...
> 	for (i = 0; i < cxld->interleave_ways; i++)
> 		cxld->target_map[i] = target_list.target_id[i];
> ...
> }
> 
> Since target_list.target_id is an 8-byte stack-allocated union, if
> interleave_ways is larger than 8 (e.g., 16), could this loop read past
> the end of the 8-byte array and leak adjacent stack memory into
> cxld->target_map?
> 
> > @@ -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/20260711180755.1779002-1-alison.schofield@intel.com?part=1
> 

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

* Re: [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path
  2026-07-11 18:07 [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path Alison Schofield
  2026-07-11 18:19 ` sashiko-bot
@ 2026-07-13  2:48 ` Li Ming
  1 sibling, 0 replies; 4+ messages in thread
From: Li Ming @ 2026-07-13  2:48 UTC (permalink / raw)
  To: Alison Schofield
  Cc: linux-cxl, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Vishal Verma, Ira Weiny, Dan Williams

On 7/12/2026 2:07 AM, Alison Schofield wrote:
> 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.
>
> Found via inspection.
>
> Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  drivers/cxl/core/region.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b6..c86dd9d55e93 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;
>  		}
>  	}
> -	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);
> +

Is it better to use a __free(put_device) instead of this open-coded?

Ming

>  	return rc;
>  }
>  
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482



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

end of thread, other threads:[~2026-07-13  2:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-11 18:07 [PATCH] cxl/region: Fix use-after-free in find_pos_and_ways() error path Alison Schofield
2026-07-11 18:19 ` sashiko-bot
2026-07-11 19:52   ` Alison Schofield
2026-07-13  2:48 ` Li Ming

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