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: <aQ3NHnL2rF0wkqeo@smile.fi.intel.com>
Date: Fri, 7 Nov 2025 12:42:38 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Ma Ke <make24@...as.ac.cn>, 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 07, 2025 at 10:26:10AM +0000, Nuno Sá wrote:
> On Fri, 2025-11-07 at 10:02 +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.

...

> > -free_descs:
> > -	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
> >  free_trig:
> > -	kfree(trig);
> > +	put_device(&trig->dev);
> 
> Yes, device_initialize() docs do say that we should give the reference instead of
> freeing the device but I'm not see how that helps in here. Maybe initializing the
> device should be done only after all the resources are allocated so the code is a bit
> more clear... But doing it like you're doing just means that we might get into the
> release function with things that might or might not be allocated which is a pattern
> I would prefer to avoid.

The put_device() here is the correct (and must) thing to do independently on
the preferences. The problem is that device_initialise() and followed calls
may do much more than just some initialisation.


-- 
With Best Regards,
Andy Shevchenko