[PATCH] device-dax: Fix refcount leak in __devm_create_dev_dax() error path

[PATCH] device-dax: Fix refcount leak in __devm_create_dev_dax() error path

[Guangshuo Li]

After device_initialize(), the lifetime of the embedded struct device is
expected to be managed through the device core reference counting.

In __devm_create_dev_dax(), several failure paths after
device_initialize() free dev_dax directly instead of releasing the
device reference with put_device(). This bypasses the normal device
lifetime rules and may leave the reference count of the embedded struct
device unbalanced, resulting in a refcount leak and potentially leading
to a use-after-free.

Fix this by assigning dev->type before device_initialize(), so the
release callback is available for put_device(), and use put_device() in
the post-initialization error paths. Keep dev_dax range cleanup explicit
in the error path.

Fixes: c2f3011ee697f ("device-dax: add an allocation interface for device-dax instances")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/dax/bus.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index fde29e0ad68b..8753115cd371 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -1453,6 +1453,7 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
 	}
 
 	dev = &dev_dax->dev;
+	dev->type = &dev_dax_type;
 	device_initialize(dev);
 	dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id);
 
@@ -1499,7 +1500,6 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
 	dev->devt = inode->i_rdev;
 	dev->bus = &dax_bus_type;
 	dev->parent = parent;
-	dev->type = &dev_dax_type;
 
 	rc = device_add(dev);
 	if (rc) {
@@ -1523,14 +1523,21 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
 
 err_alloc_dax:
 	kfree(dev_dax->pgmap);
+	dev_dax->pgmap = NULL;
+
 err_pgmap:
 	free_dev_dax_ranges(dev_dax);
+	put_device(dev);
+	return ERR_PTR(rc);
+
 err_range:
-	free_dev_dax_id(dev_dax);
+	put_device(dev);
+	return ERR_PTR(rc);
+
 err_id:
 	kfree(dev_dax);
-
 	return ERR_PTR(rc);
+
 }
 
 struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data)
-- 
2.43.0

Re: [PATCH] device-dax: Fix refcount leak in __devm_create_dev_dax() error path

[Dan Williams]

Guangshuo Li wrote:
> After device_initialize(), the lifetime of the embedded struct device is
> expected to be managed through the device core reference counting.
> 
> In __devm_create_dev_dax(), several failure paths after
> device_initialize() free dev_dax directly instead of releasing the
> device reference with put_device(). This bypasses the normal device
> lifetime rules and may leave the reference count of the embedded struct
> device unbalanced, resulting in a refcount leak and potentially leading
> to a use-after-free.

Please do not list "theoretical" problems as justification. Point to
real problems.

> Fix this by assigning dev->type before device_initialize(), so the
> release callback is available for put_device(), and use put_device() in
> the post-initialization error paths. Keep dev_dax range cleanup explicit
> in the error path.

I see a more straightforward way to address just the practical problem
that also incorporates the other feedback I have below. Can you spot
that and fixup the changelog to address the practical impact?

> Fixes: c2f3011ee697f ("device-dax: add an allocation interface for device-dax instances")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/dax/bus.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index fde29e0ad68b..8753115cd371 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -1453,6 +1453,7 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
>  	}
>  
>  	dev = &dev_dax->dev;
> +	dev->type = &dev_dax_type;
>  	device_initialize(dev);
>  	dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id);
>  
> @@ -1499,7 +1500,6 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
>  	dev->devt = inode->i_rdev;
>  	dev->bus = &dax_bus_type;
>  	dev->parent = parent;
> -	dev->type = &dev_dax_type;
>  
>  	rc = device_add(dev);
>  	if (rc) {
> @@ -1523,14 +1523,21 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
>  
>  err_alloc_dax:
>  	kfree(dev_dax->pgmap);
> +	dev_dax->pgmap = NULL;
> +
>  err_pgmap:
>  	free_dev_dax_ranges(dev_dax);
> +	put_device(dev);
> +	return ERR_PTR(rc);
> +
>  err_range:
> -	free_dev_dax_id(dev_dax);
> +	put_device(dev);
> +	return ERR_PTR(rc);

Please no gotos with early returns, that makes a mess.

Re: [PATCH] device-dax: Fix refcount leak in __devm_create_dev_dax() error path

[Guangshuo Li]

Hi Dan,

Thank you for the review and for pointing that out.

You are right that my changelog overstated the impact. I do not have a
concrete use-after-free case here, and the practical issue is simply
that after device_initialize(), the embedded struct device should be
released through the device core with put_device(), rather than
freeing dev_dax directly.

I also took a closer look at the release path. Since dev_dax_release()
already handles free_dev_dax_id(), kfree(dev_dax->pgmap), and
kfree(dev_dax), and put_dax() is NULL-safe, the post-initialization
failure paths can be simplified to explicit range cleanup plus
put_device(), once dev->type is assigned before device_initialize().

I'll send a v2 that tightens the changelog around the actual lifecycle
issue and cleans up the error paths accordingly.

Thanks again for the guidance.

Best regards,
Guangshuo

Dan Williams <djbw@kernel.org> 于2026年4月12日周日 07:29写道：
>
> Guangshuo Li wrote:
> > After device_initialize(), the lifetime of the embedded struct device is
> > expected to be managed through the device core reference counting.
> >
> > In __devm_create_dev_dax(), several failure paths after
> > device_initialize() free dev_dax directly instead of releasing the
> > device reference with put_device(). This bypasses the normal device
> > lifetime rules and may leave the reference count of the embedded struct
> > device unbalanced, resulting in a refcount leak and potentially leading
> > to a use-after-free.
>
> Please do not list "theoretical" problems as justification. Point to
> real problems.
>
> > Fix this by assigning dev->type before device_initialize(), so the
> > release callback is available for put_device(), and use put_device() in
> > the post-initialization error paths. Keep dev_dax range cleanup explicit
> > in the error path.
>
> I see a more straightforward way to address just the practical problem
> that also incorporates the other feedback I have below. Can you spot
> that and fixup the changelog to address the practical impact?
>
> > Fixes: c2f3011ee697f ("device-dax: add an allocation interface for device-dax instances")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> >  drivers/dax/bus.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index fde29e0ad68b..8753115cd371 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -1453,6 +1453,7 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
> >       }
> >
> >       dev = &dev_dax->dev;
> > +     dev->type = &dev_dax_type;
> >       device_initialize(dev);
> >       dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id);
> >
> > @@ -1499,7 +1500,6 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
> >       dev->devt = inode->i_rdev;
> >       dev->bus = &dax_bus_type;
> >       dev->parent = parent;
> > -     dev->type = &dev_dax_type;
> >
> >       rc = device_add(dev);
> >       if (rc) {
> > @@ -1523,14 +1523,21 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
> >
> >  err_alloc_dax:
> >       kfree(dev_dax->pgmap);
> > +     dev_dax->pgmap = NULL;
> > +
> >  err_pgmap:
> >       free_dev_dax_ranges(dev_dax);
> > +     put_device(dev);
> > +     return ERR_PTR(rc);
> > +
> >  err_range:
> > -     free_dev_dax_id(dev_dax);
> > +     put_device(dev);
> > +     return ERR_PTR(rc);
>
> Please no gotos with early returns, that makes a mess.