linux-kernel - Re: [PATCH v3] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VfiKrCC-T45z4RggH0-eMnxvRW2pGC85KWZDdoaxNALDw@mail.gmail.com>
Date: Fri, 7 Nov 2025 10:03:43 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Ma Ke <make24@...as.ac.cn>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com, 
	andy@...nel.org, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	akpm@...ux-foundation.org
Subject: Re: [PATCH v3] iio: trigger: Fix error handling in viio_trigger_alloc

On Fri, Nov 7, 2025 at 4:02 AM Ma Ke <make24@...as.ac.cn> 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.
> 2. The release function may attempt to free negative subirq_base.
> 3. Missing mutex_destroy() in release function.
>
> Fix these issues by:
> 1. Replacing kfree(trig) with put_device(&trig->dev) in error paths.
> 2. Removing the free_descs label and handling IRQ descriptor freeing
>    directly in the kvasprintf() error path.
> 3. Adding missing mutex_destroy().
>
> Found by code review.

...

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

> +               irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);

Why?

> +               goto free_trig;
> +       }

Please, slow down and think a bit. Also you may split the patch to at least two:
1) add missed mutex_destroy();
2) fix the clean up in the error path.


-- 
With Best Regards,
Andy Shevchenko