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@xxxxxxxxxxxxxxx
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>  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;
>  }