[PATCH] iio: trigger: Fix refcount leak in viio_trigger_alloc() error path

[PATCH] iio: trigger: Fix refcount leak in viio_trigger_alloc() 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 viio_trigger_alloc(), if irq_alloc_descs() or kvasprintf() fails,
the error path frees trig directly with kfree() rather than 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 using put_device(&trig->dev) in the failure path and let
iio_trig_release() handle the final cleanup. Also update the subirq_base
check in iio_trig_release() to test for >= 0, so that a negative error
code from irq_alloc_descs() is not treated as a valid IRQ descriptor
base during cleanup.

Fixes: 2c99f1a09da3 ("iio: trigger: clean up viio_trigger_alloc()")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/iio/industrialio-trigger.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..ab544976018f 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -509,7 +509,7 @@ static void iio_trig_release(struct device *device)
 	struct iio_trigger *trig = to_iio_trigger(device);
 	int i;
 
-	if (trig->subirq_base) {
+	if (trig->subirq_base >= 0) {
 		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
 			irq_modify_status(trig->subirq_base + i,
 					  IRQ_NOAUTOEN,
@@ -572,11 +572,11 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 					    CONFIG_IIO_CONSUMERS_PER_TRIGGER,
 					    0);
 	if (trig->subirq_base < 0)
-		goto free_trig;
+		goto err_put;
 
 	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
 	if (trig->name == NULL)
-		goto free_descs;
+		goto err_put;
 
 	INIT_LIST_HEAD(&trig->list);
 
@@ -594,10 +594,8 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 
 	return trig;
 
-free_descs:
-	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
-free_trig:
-	kfree(trig);
+err_put:
+	put_device(&trig->dev);
 	return NULL;
 }
 
-- 
2.43.0

Re: [PATCH] iio: trigger: Fix refcount leak in viio_trigger_alloc() error path

[Dan Carpenter]

On Sat, Apr 11, 2026 at 04:04:35PM +0800, Guangshuo Li wrote:
> After device_initialize(), the lifetime of the embedded struct device
  ^^^^^
The commit message says after but you're changing the before.

> is expected to be managed through the device core reference counting.
> 
> In viio_trigger_alloc(), if irq_alloc_descs() or kvasprintf() fails,
> the error path frees trig directly with kfree() rather than 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 using put_device(&trig->dev) in the failure path and let
> iio_trig_release() handle the final cleanup. Also update the subirq_base
> check in iio_trig_release() to test for >= 0, so that a negative error
> code from irq_alloc_descs() is not treated as a valid IRQ descriptor
> base during cleanup.
> 
> Fixes: 2c99f1a09da3 ("iio: trigger: clean up viio_trigger_alloc()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/iio/industrialio-trigger.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 54416a384232..ab544976018f 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -509,7 +509,7 @@ static void iio_trig_release(struct device *device)
>  	struct iio_trigger *trig = to_iio_trigger(device);
>  	int i;
>  
> -	if (trig->subirq_base) {
> +	if (trig->subirq_base >= 0) {
>  		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
>  			irq_modify_status(trig->subirq_base + i,
>  					  IRQ_NOAUTOEN,
> @@ -572,11 +572,11 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>  					    CONFIG_IIO_CONSUMERS_PER_TRIGGER,
>  					    0);
>  	if (trig->subirq_base < 0)
> -		goto free_trig;
> +		goto err_put;
>  
>  	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
>  	if (trig->name == NULL)
> -		goto free_descs;
> +		goto err_put;

At this point we haven't done:

	trig->dev.type = &iio_trig_type;
or
	device_initialize(&trig->dev);

So the original code is fine and the new code just introduces memory
leaks.

regards,
dan carpenter

>  
>  	INIT_LIST_HEAD(&trig->list);
>  
> @@ -594,10 +594,8 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>  
>  	return trig;
>  
> -free_descs:
> -	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
> -free_trig:
> -	kfree(trig);
> +err_put:
> +	put_device(&trig->dev);
>  	return NULL;
>  }

Re: [PATCH] iio: trigger: Fix refcount leak in viio_trigger_alloc() error path

[Guangshuo Li]

Hi Dan,

Thank you very much for your review and for pointing this out.

The kernel version on our side is `v6.19-rc8-214-ge7aa57247700`. For
clarity, below is the full `viio_trigger_alloc()` function in our
tree:

```c
struct iio_trigger *viio_trigger_alloc(struct device *parent,
       struct module *this_mod,
       const char *fmt,
       va_list vargs)
{
struct iio_trigger *trig;
int i;

trig = kzalloc(sizeof(*trig), GFP_KERNEL);
if (!trig)
return NULL;

trig->dev.parent = parent;
trig->dev.type = &iio_trig_type;
trig->dev.bus = &iio_bus_type;
device_initialize(&trig->dev);
INIT_WORK(&trig->reenable_work, iio_reenable_work_fn);

mutex_init(&trig->pool_lock);
trig->subirq_base = irq_alloc_descs(-1, 0,
    CONFIG_IIO_CONSUMERS_PER_TRIGGER,
    0);
if (trig->subirq_base < 0)
goto free_trig;

trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
if (trig->name == NULL)
goto free_descs;

INIT_LIST_HEAD(&trig->list);

trig->owner = this_mod;

trig->subirq_chip.name = trig->name;
trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
irq_set_chip(trig->subirq_base + i, &trig->subirq_chip);
irq_set_handler(trig->subirq_base + i, &handle_simple_irq);
irq_modify_status(trig->subirq_base + i,
  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
}

return trig;

free_descs:
irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
free_trig:
kfree(trig);
return NULL;
}
```

So in this version, both error paths are reached after `device_initialize()`.

That was why I thought `put_device(&trig->dev)` would be more
appropriate here than freeing `trig` directly with `kfree()`.

Also, since `irq_alloc_descs()` can return a negative error code, I
thought changing the release-side check to `trig->subirq_base >= 0`
was needed as well.

I may be missing something here, so I would very much appreciate any
correction if my understanding is off.

Best regards,
Guangshuo

Re: [PATCH] iio: trigger: Fix refcount leak in viio_trigger_alloc() error path

[Dan Carpenter]

Ah, yes.  It *was* a real bug but it was fixed a few months ago
by commit 12b393486c70 ("iio: core: Clean up device correctly on
viio_trigger_alloc() failure").

It's unfortunate that that commit didn't have a Fixes tag.

regards,
dan carpenter