Re: [PATCH] iio: gyro: mpu3050: Fix resource leak

[Jonathan Cameron]

On Sun, 22 Feb 2026 13:07:26 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Fri, Feb 20, 2026 at 02:05:22PM -0600, Ethan Tidmore wrote:
> > The interrupt handler is setup but only a few lines down if
> > iio_trigger_register() fails the function returns without properly
> > releasing the handler:
> > 
> > 	ret = request_threaded_irq(irq,
> > 				   mpu3050_irq_handler,
> > 				   mpu3050_irq_thread,
> > 				   irq_trig,
> > 				   mpu3050->trig->name,
> > 				   mpu3050->trig);
> > 
> > ...
> > 
> > 	ret = iio_trigger_register(mpu3050->trig);
> > 	if (ret)
> > 		return ret;
> > 
> > 	indio_dev->trig = iio_trigger_get(mpu3050->trig);
> > 
> > 	return 0;
> > }
> > 
> > Change request_threaded_irq() to devm_request_threaded_irq() to resolve
> > resource leak and use current API.
> > 
> > Detected by Smatch:
> > drivers/iio/gyro/mpu3050-core.c:1128 mpu3050_trigger_probe() warn:
> > 'irq' from request_threaded_irq() not released on lines: 1124.  
> 
> ...
> 
> > +	ret = devm_request_threaded_irq(dev,
> > +					irq,
> > +					mpu3050_irq_handler,
> > +					mpu3050_irq_thread,
> > +					irq_trig,
> > +					mpu3050->trig->name,
> > +					mpu3050->trig);
> >  	if (ret) {
> >  		dev_err(dev, "can't get IRQ %d, error %d\n", irq, ret);
> >  		return ret;  
> 
> Now this prints two messages on the error path...
> 

This driver is a bit of a mess wrt to devm already and the change here makes
things worse.  Rule for devm (to make it easy to reason about) is you use it
on everything until a particular point in probe when you switch to not using
it for anything.   Mix and match leads to subtle race conditions.

Now, today the mix and match is there but on memory allocations which tend
to be safe to release late (though not recommended to do so!) this adds
irqs to that mix.

So your patch is fixing a real issue, but I think we should be looking to
clean up the other related issues around it.

As a side note, the indio_dev->trig should not be set after iio_device_register()
That's one reason almost all drivers register triggers before the device.

Also the free_irq() you are dropping is out of sequence wrt to
iio_triggered_buffer_cleanup()

If you want to take this driver on I'd suggest it needs a more complex and general
conversion to devm.

Alternative is just fix the problem you've identified but not using devm_
but instead use goto and a manual free_irq() call in the error path.


Thanks,

Jonathan