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

On Wed, Jul 22, 2026 at 12:53:15AM +0100, Jonathan Cameron wrote:
> On Tue, 21 Jul 2026 10:37:03 -0700
> Alison Schofield <[email protected]> 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 <[email protected]>
> 
> 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 <[email protected]>

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
>