linux-kernel - Re: [PATCH v2] iio: trigger: fix use-after-free in viio_trigger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260207153546.74d4fc2b@jic23-huawei>
Date: Sat, 7 Feb 2026 15:35:46 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Salah Triki <salah.triki@...il.com>
Cc: David Lechner <dlechner@...libre.com>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: trigger: fix use-after-free in
 viio_trigger_alloc()

On Wed,  4 Feb 2026 21:34:13 +0100
Salah Triki <salah.triki@...il.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@...il.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;
>  }
>