[PATCH v2] iio: trigger: fix use-after-free in viio_trigger_alloc()

[PATCH v2] iio: trigger: fix use-after-free in viio_trigger_alloc()

[Salah Triki]

Once `device_initialize()` is called, the reference count of the device is
set to 1. The memory associated with the device must then be managed by
the kobject reference counting via `put_device()`.

Currently, if `irq_alloc_descs()` or `kvasprintf()` fails, the code
manually calls `irq_free_descs()` and `kfree()`. This is problematic for
two reasons:

1. Calling `kfree()` directly bypasses the device's release callback
   (`iio_trig_release()`), which could lead to resource leaks or
   inconsistencies within the driver core.

2. If we simply replace `kfree()` with `put_device()`, a double free
   occurs because `iio_trig_release()` already calls `irq_free_descs()`.

Fix this by:
- Using `put_device()` to handle memory tearing down.
- Removing the manual call to `irq_free_descs()` in the error path, as
  it is already handled by the trigger's release function.

Path to the issue:
viio_trigger_alloc()
  -> device_initialize() (refcount = 1)
  -> kvasprintf() fails
  -> goto free_descs
  -> irq_free_descs() (first manual free)
  -> kfree(trig) (refcount is still 1, release never called)

Fixes: 2c99f1a09da3d ("iio: trigger: clean up viio_trigger_alloc()")
Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes in v2:
- Remove the manual call to irq_free_descs() in the error path to avoid 
  a double free, as this is already handled by iio_trig_release().
- Clarify the error path and the potential for memory corruption in 
  the commit description.
- Remove the blank line in the tag block to comply with kernel script 
  requirements.

 drivers/iio/industrialio-trigger.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..7f53e2a5a101 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -576,7 +576,7 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
 
 	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
 	if (trig->name == NULL)
-		goto free_descs;
+		goto free_trig;
 
 	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);
+	put_device(&trig->dev);
 	return NULL;
 }
 
-- 
2.43.0

Re: [PATCH v2] iio: trigger: fix use-after-free in viio_trigger_alloc()

[Jonathan Cameron]

On Wed,  4 Feb 2026 21:34:13 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> Once `device_initialize()` is called, the reference count of the device is
> set to 1. The memory associated with the device must then be managed by
> the kobject reference counting via `put_device()`.
Hi Salah,

This explanation has become a little unnecessarily long and unclear.
I think it needs a rewrite.

> 
> Currently, if `irq_alloc_descs()` or `kvasprintf()` fails, the code
> manually calls `irq_free_descs()` and `kfree()`. This is problematic for
> two reasons:
> 
> 1. Calling `kfree()` directly bypasses the device's release callback
>    (`iio_trig_release()`), which could lead to resource leaks or
>    inconsistencies within the driver core.

Does it?  A could doesn't provide information on whether to back port or
not.  If you are going to make this statement, make it clear what
leak or meaningful inconsistency occurs.

I would instead pitch this as following a standard design pattern and
reducing the duplication of code for the cleanup path.

> 
> 2. If we simply replace `kfree()` with `put_device()`, a double free
>    occurs because `iio_trig_release()` already calls `irq_free_descs()`.
I don't thing this needs calling out explicitly as it's kind
of more about what was wrong in previous version than something
that needs stating here.


> 
> Fix this by:
> - Using `put_device()` to handle memory tearing down.
> - Removing the manual call to `irq_free_descs()` in the error path, as
>   it is already handled by the trigger's release function.
Simplify this to something like:

Use put_device() which removes the need to cal irq_free_descs()
explicitly in the error path as that is also also part of the
release function.


> 
> Path to the issue:
> viio_trigger_alloc()
>   -> device_initialize() (refcount = 1)
>   -> kvasprintf() fails
>   -> goto free_descs
>   -> irq_free_descs() (first manual free)
>   -> kfree(trig) (refcount is still 1, release never called)  
This bit is unnecessary detail.

> 
> Fixes: 2c99f1a09da3d ("iio: trigger: clean up viio_trigger_alloc()")
Without a clear statement of the bug (and refcount == 1 on something
we kfree is inelegant but not a bug) then a fixes tag is not appropriate.
It triggers backports, which may or may not make sense for this.

I'll note the minimal fix (which is less good for other reasons) is
simply do the device_initialize() later after we are sure we won't get
a failure.

> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
> Changes in v2:
> - Remove the manual call to irq_free_descs() in the error path to avoid 
>   a double free, as this is already handled by iio_trig_release().
> - Clarify the error path and the potential for memory corruption in 
>   the commit description.
> - Remove the blank line in the tag block to comply with kernel script 
>   requirements.
> 
>  drivers/iio/industrialio-trigger.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 54416a384232..7f53e2a5a101 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -576,7 +576,7 @@ struct iio_trigger *viio_trigger_alloc(struct device *parent,
>  
>  	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
>  	if (trig->name == NULL)
> -		goto free_descs;
> +		goto free_trig;
>  
>  	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);
> +	put_device(&trig->dev);
>  	return NULL;
>  }
>  

Re: [PATCH v2] iio: trigger: fix use-after-free in viio_trigger_alloc()

[Salah Triki]

Hi Jonathan,

You're right, the explanation was too wordy. I'll send a v3 focusing on the
fact that using put_device() follows the standard design pattern and avoids
duplicating the cleanup logic already present in the release callback.
I will also remove the Fixes tag as this is more of a robustness 
improvement than a critical bug fix.

Best regards,

Salah