[PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[Alison Schofield]

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.

Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

No changes in v2.
Reposted in series w the __free change in patch 2/2



 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

[PATCH v2 2/2] cxl/region: Use __free(put_device) in find_pos_and_ways()

[Alison Schofield]

Use __free(put_device) for the switch decoder reference returned by
device_find_child() instead of releasing it with an open-coded
put_device().

This matches the scoped device reference handling used elsewhere in
the file.

Suggested-by: Li Ming <ming.li@zohomail.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

This is a new patch added in series after the UAF fixup in patch 1/2.


 drivers/cxl/core/region.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index c86dd9d55e93..4d7d96d03ece 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1911,9 +1911,9 @@ static int match_switch_decoder_by_range(struct device *dev,
 static int find_pos_and_ways(struct cxl_port *port, struct range *range,
 			     int *pos, int *ways)
 {
+	struct device *dev __free(put_device) = NULL;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_port *parent;
-	struct device *dev;
 	int rc = -ENXIO;
 
 	parent = parent_port_of(port);
@@ -1945,8 +1945,6 @@ 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;
 }
 
-- 
2.37.3

Re: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware-provided interleave ways can exceed switch decoder target count, causing out-of-bounds heap writes.
--

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

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

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.

> 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,
>  			break;
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but I have a question about the loop that
concludes here.

In drivers/cxl/core/region.c:find_pos_and_ways(), the code iterates up to the
interleave_ways read from the switch decoder:

	*ways = cxlsd->cxld.interleave_ways;

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

Is it possible for the hardware-provided interleave_ways to exceed the switch
decoder's actual target count (cxlsd->nr_targets)?

If the hardware control register specifies an interleave_ways value that is
larger than the capability used to size the flexible array, could this loop
read out-of-bounds from the cxlsd->target[] array?

Similarly, in drivers/cxl/core/port.c:decoder_populate_targets(), could this
mismatch also result in an out-of-bounds write when populating targets from
hardware registers without bounds checking against nr_targets?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/3a36953b5c665cb9544ba61dafbea0b0fa508c77.1784654628.git.alison.schofield@intel.com?part=1

Re: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[Jonathan Cameron]

On Tue, 21 Jul 2026 10:37:03 -0700
Alison Schofield <alison.schofield@intel.com> 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.

Bonus space before "the"

> 
> Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Fix is good but needs more analysis than I'd like because two different
paths to the same struct device. 

What follows is very much a suggestion so if you disagree I'm also
fine with this fix going in on it's own.

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

> ---
> 
> No changes in v2.
> Reposted in series w the __free change in patch 2/2
> 
> 
> 
>  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));

It's not obvious from that error print which device is which.
Maybe the print should therefore being using dev?  It's the last
line that matters.

Might need a rename to make it clear it's the switch decoder dev to
make this all really clear.  swd_dev maybe?  Or just take the view
the code is local enough and keep name as dev but switch this to

			dev_name(dev));



>  
> +	put_device(dev);
> +
>  	return rc;
>  }
>  
> 
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482

Re: [PATCH v2 2/2] cxl/region: Use __free(put_device) in find_pos_and_ways()

[Jonathan Cameron]

On Tue, 21 Jul 2026 10:37:04 -0700
Alison Schofield <alison.schofield@intel.com> wrote:

> Use __free(put_device) for the switch decoder reference returned by
> device_find_child() instead of releasing it with an open-coded
> put_device().
> 
> This matches the scoped device reference handling used elsewhere in
> the file.
> 
> Suggested-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
> 
> This is a new patch added in series after the UAF fixup in patch 1/2.
> 
> 
>  drivers/cxl/core/region.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c86dd9d55e93..4d7d96d03ece 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1911,9 +1911,9 @@ static int match_switch_decoder_by_range(struct device *dev,
>  static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  			     int *pos, int *ways)
>  {
> +	struct device *dev __free(put_device) = NULL;

No to this.  See rules in cleanup.h.
This is the path to grumpy Linus if he happens to spot it.

Short story is declaration and destructor must be in same statement
as the constructor.  Here that means 

	struct device *dev __free(put_device) =
		device_find_child(&parent->dev, range, match_switch_decoder_by_range);

I would go long on that second line given it's only a bit beyond 80 chars
and the two lines are horrible enough to read without becoming 3.



>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_port *parent;
> -	struct device *dev;
>  	int rc = -ENXIO;
>  
>  	parent = parent_port_of(port);
> @@ -1945,8 +1945,6 @@ 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;
>  }
>  

Re: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[Li Ming]

在 2026/7/22 01:37, Alison Schofield 写道:
> 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.
>
> Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Li Ming <ming.li@zohomail.com>


> ---
>
> No changes in v2.
> Reposted in series w the __free change in patch 2/2
>
>
>
>   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

Re: [PATCH v2 2/2] cxl/region: Use __free(put_device) in find_pos_and_ways()

[Li Ming]

在 2026/7/22 01:37, Alison Schofield 写道:
> Use __free(put_device) for the switch decoder reference returned by
> device_find_child() instead of releasing it with an open-coded
> put_device().
>
> This matches the scoped device reference handling used elsewhere in
> the file.
>
> Suggested-by: Li Ming <ming.li@zohomail.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Li Ming <ming.li@zohomail.com>


> ---
>
> This is a new patch added in series after the UAF fixup in patch 1/2.
>
>
>   drivers/cxl/core/region.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index c86dd9d55e93..4d7d96d03ece 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1911,9 +1911,9 @@ static int match_switch_decoder_by_range(struct device *dev,
>   static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>   			     int *pos, int *ways)
>   {
> +	struct device *dev __free(put_device) = NULL;
>   	struct cxl_switch_decoder *cxlsd;
>   	struct cxl_port *parent;
> -	struct device *dev;
>   	int rc = -ENXIO;
>   
>   	parent = parent_port_of(port);
> @@ -1945,8 +1945,6 @@ 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;
>   }
>   

Re: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[Alison Schofield]

On Tue, Jul 21, 2026 at 05:50:32PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Hardware-provided interleave ways can exceed switch decoder target count, causing out-of-bounds heap writes.
> --

This is being worked in a series w other Sashiko issues called
'Harden HDM decoder enumeration'

https://lore.kernel.org/linux-cxl/cover.1784595445.git.alison.schofield@intel.com/

Re: [PATCH v2 1/2] cxl/region: Fix use-after-free in find_pos_and_ways() error path

[Alison Schofield]

On Wed, Jul 22, 2026 at 12:53:15AM +0100, Jonathan Cameron wrote:
> On Tue, 21 Jul 2026 10:37:03 -0700
> Alison Schofield <alison.schofield@intel.com> 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.
> 
> Bonus space before "the"

Bonus rescinded

> 
> > 
> > Fixes: d90acdf49e18 ("cxl/region: Add a dev_err() on missing target list entries")
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> 
> Fix is good but needs more analysis than I'd like because two different
> paths to the same struct device. 
> 
> What follows is very much a suggestion so if you disagree I'm also
> fine with this fix going in on it's own.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Thanks for the review.
A change to dev_name(dev), see below, folds those two paths into one.

> 
> > ---
> > 
> > No changes in v2.
> > Reposted in series w the __free change in patch 2/2
> > 
> > 
> > 
> >  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));
> 
> It's not obvious from that error print which device is which.
> Maybe the print should therefore being using dev?  It's the last
> line that matters.
> 
> Might need a rename to make it clear it's the switch decoder dev to
> make this all really clear.  swd_dev maybe?  Or just take the view
> the code is local enough and keep name as dev but switch this to
> 
> 			dev_name(dev));

Yes, let's make that clearer.
dev and &cxlsd->cxld.dev are the same device, so the output
is unchanged, but dev_name(dev) makes it clear which device is logged.
I kept the name dev:
	dev_name(port->parent_dport->dport_dev), dev_name(dev));

-- Alison
> 

Re: [PATCH v2 2/2] cxl/region: Use __free(put_device) in find_pos_and_ways()

[Alison Schofield]

On Wed, Jul 22, 2026 at 12:56:48AM +0100, Jonathan Cameron wrote:
> On Tue, 21 Jul 2026 10:37:04 -0700
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > Use __free(put_device) for the switch decoder reference returned by
> > device_find_child() instead of releasing it with an open-coded
> > put_device().
> > 
> > This matches the scoped device reference handling used elsewhere in
> > the file.
> > 
> > Suggested-by: Li Ming <ming.li@zohomail.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> > 
> > This is a new patch added in series after the UAF fixup in patch 1/2.
> > 
> > 
> >  drivers/cxl/core/region.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index c86dd9d55e93..4d7d96d03ece 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1911,9 +1911,9 @@ static int match_switch_decoder_by_range(struct device *dev,
> >  static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> >  			     int *pos, int *ways)
> >  {
> > +	struct device *dev __free(put_device) = NULL;
> 
> No to this.  See rules in cleanup.h.
> This is the path to grumpy Linus if he happens to spot it.
> 
> Short story is declaration and destructor must be in same statement
> as the constructor.  Here that means 
> 
> 	struct device *dev __free(put_device) =
> 		device_find_child(&parent->dev, range, match_switch_decoder_by_range);
> 
> I would go long on that second line given it's only a bit beyond 80 chars
> and the two lines are horrible enough to read without becoming 3.

Thanks for the cleanup.h pointer.
In v3 dev is declared and assigned in a single statement after the
!parent check, formatted as you suggested above.

-- Alison