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

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

[Guangshuo Li]

After device_initialize(), the embedded struct device in dev_dax is
expected to be released through the device core with put_device().

In __devm_create_dev_dax(), several failure paths after
device_initialize() free dev_dax directly instead of dropping the device
reference, which bypasses the normal device core lifetime handling and
leaks the reference held on the embedded struct device.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fix this by assigning dev->type before device_initialize(), so the
release callback is available, use put_device() in the
post-initialization error paths, and keep dev_dax range cleanup explicit
since it is not handled by dev_dax_release().

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>
---
v3:
  - note that the issue was identified by my static analysis tool
  - and confirmed by manual review

v2:
  - clarify the commit message around the device reference leak
  - drop the unsupported use-after-free claim
  - set dev->type before device_initialize() so put_device() can use the
    release callback on post-init failures
  - simplify the post-initialization error paths to use explicit range
    cleanup plus put_device()

 drivers/dax/bus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index fde29e0ad68b..2d92674d0d6e 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) {
@@ -1522,14 +1522,13 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
 	return dev_dax;
 
 err_alloc_dax:
-	kfree(dev_dax->pgmap);
 err_pgmap:
 	free_dev_dax_ranges(dev_dax);
 err_range:
-	free_dev_dax_id(dev_dax);
+	put_device(dev);
+	return ERR_PTR(rc);
 err_id:
 	kfree(dev_dax);
-
 	return ERR_PTR(rc);
 }
 
-- 
2.43.0

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

[Jonathan Cameron]

On Mon, 13 Apr 2026 21:56:25 +0800
Guangshuo Li <lgs201920130244@gmail.com> wrote:

> After device_initialize(), the embedded struct device in dev_dax is
> expected to be released through the device core with put_device().
> 
> In __devm_create_dev_dax(), several failure paths after
> device_initialize() free dev_dax directly instead of dropping the device
> reference, which bypasses the normal device core lifetime handling and
> leaks the reference held on the embedded struct device.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
> 
> Fix this by assigning dev->type before device_initialize(), so the
> release callback is available, use put_device() in the
> post-initialization error paths, and keep dev_dax range cleanup explicit
> since it is not handled by dev_dax_release().
> 
> 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>
Hi

Whilst I think your fix is correct the need to still handle one error path
via a different set of goto labels is not as easy to read as I'd like to see.

There is also some ordering stuff in here that is somewhat messy and
needs some more thought.  alloc_dev_dax_range() is unwound out of order
wrt to data->pgmap.

Thanks,

Jonathan

> ---
> v3:
>   - note that the issue was identified by my static analysis tool
>   - and confirmed by manual review
> 
> v2:
>   - clarify the commit message around the device reference leak
>   - drop the unsupported use-after-free claim
>   - set dev->type before device_initialize() so put_device() can use the
>     release callback on post-init failures
>   - simplify the post-initialization error paths to use explicit range
>     cleanup plus put_device()
> 
>  drivers/dax/bus.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index fde29e0ad68b..2d92674d0d6e 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) {
> @@ -1522,14 +1522,13 @@ static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
>  	return dev_dax;
>  
>  err_alloc_dax:
> -	kfree(dev_dax->pgmap);
>  err_pgmap:
>  	free_dev_dax_ranges(dev_dax);
This bothers me somewhat as now the error paths are not unwinding in reverse of the setup.

>  err_range:
> -	free_dev_dax_id(dev_dax);
> +	put_device(dev);
> +	return ERR_PTR(rc);
It would be helpful to have some white space around this to make it easier
to spot given it's in the middle of the list.


>  err_id:
>  	kfree(dev_dax);
Cam we juggle things around a little more so that there is no path that
hits this?  I.e. move device_initialize() (and whatever needs setting for
release to work) to just after allocation?

I think the one complication is we need to ensure correct behavior if the
id has not been successfully allocated.  Perhaps using a flag value of -1 would
make this easy to check for.

Alternative would be to have a helper that does the allocate and ID allocation
parts and handles this kfree() internally.


> -
Unrelated and probably not a desirable change.  It's common to put a blank line
before simple returns like this one.

>  	return ERR_PTR(rc);
>  }
>  

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

[Dan Williams]

On Mon, Apr 13, 2026, at 6:56 AM, Guangshuo Li wrote:
> After device_initialize(), the embedded struct device in dev_dax is
> expected to be released through the device core with put_device().
>
> In __devm_create_dev_dax(), several failure paths after
> device_initialize() free dev_dax directly instead of dropping the device
> reference, which bypasses the normal device core lifetime handling and
> leaks the reference held on the embedded struct device.

Like I said before please focus on the practical problem this causes. It is always the case that device setup will have some steps  that are handlded by direct kfree before switching to a put_device() model.

In this case the practical problem is that the memory allocation from dev_set_name() is leaked. Also the error return from dev_set_name() is ignored.

> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.

If you are going to be doing more of these please make sure not to just rework code just to get all freeing done by put_device() when not strictly necessary.

One issue to avoid is early returns in the error goto path.

In this case I believe you can address this by moving the device_initialize() later in the function. Make it so that the switch from error unwind to put_device() is the last step of the setup.

It would be nice to fix the dev_set_name() error handling in a follow-on patch as well.