linux-kernel - Re: [PATCH v2] iio: trigger: Fix error handling 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: <aQxlxTiq59zynioS@smile.fi.intel.com>
Date: Thu, 6 Nov 2025 11:09:25 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Ma Ke <make24@...as.ac.cn>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com,
	andy@...nel.org, error27@...il.com, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	stable@...r.kernel.org
Subject: Re: [PATCH v2] iio: trigger: Fix error handling in viio_trigger_alloc

On Thu, Nov 06, 2025 at 04:29:23PM +0800, Ma Ke wrote:
> viio_trigger_alloc() initializes the device with device_initialize()
> but uses kfree() directly in error paths, which bypasses the device's
> release callback iio_trig_release(). This could lead to memory leaks
> and inconsistent device state.
> 
> Additionally, the current error handling has the following issues:
> 1. Potential double-free of IRQ descriptors when kvasprintf fails.

kvasprintf()

> 2. The release function may attempt to free negative subirq_base.
> 3. Missing mutex_destroy in release function.

mutex_destroy()

> Fix these issues by:
> 1. Replacing kfree(trig) with put_device(&trig->dev) in error paths.
> 2. Setting subirq_base to 0 after freeing IRQ descriptors in error
> path to prevent double-free in release callback.
> 3. Modifying release function to properly handle negative subirq_base.
> 4. Adding missing mutex_destroy().
> 
> Found by code review.

This is better now, but giving a nature of the issue and the fix I would really
appreciate some CIs and syzkaller (or alike) fuzzers to go with this first.

...

>  free_descs:
>  	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
> +	trig->subirq_base = 0;

Why not getting rid of this label and accompanied code altogether?

-- 
With Best Regards,
Andy Shevchenko