* [PATCH] iio: gyro: mpu3050: Fix resource leak
@ 2026-02-20 20:05 Ethan Tidmore
2026-02-22 11:07 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Ethan Tidmore @ 2026-02-20 20:05 UTC (permalink / raw)
To: linusw, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Ethan Tidmore
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.
Fixes: 3904b28efb2c7 ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
drivers/iio/gyro/mpu3050-core.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index ee2fcd20545d..eeac4ad47396 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1103,12 +1103,13 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
if (mpu3050->irq_opendrain)
irq_trig |= IRQF_SHARED;
- ret = request_threaded_irq(irq,
- mpu3050_irq_handler,
- mpu3050_irq_thread,
- irq_trig,
- mpu3050->trig->name,
- mpu3050->trig);
+ 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;
@@ -1260,8 +1261,6 @@ void mpu3050_common_remove(struct device *dev)
pm_runtime_put_noidle(dev);
pm_runtime_disable(dev);
iio_triggered_buffer_cleanup(indio_dev);
- if (mpu3050->irq)
- free_irq(mpu3050->irq, mpu3050);
iio_device_unregister(indio_dev);
mpu3050_power_down(mpu3050);
}
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix resource leak
2026-02-20 20:05 [PATCH] iio: gyro: mpu3050: Fix resource leak Ethan Tidmore
@ 2026-02-22 11:07 ` Andy Shevchenko
2026-02-22 16:30 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2026-02-22 11:07 UTC (permalink / raw)
To: Ethan Tidmore
Cc: linusw, jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel
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...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix resource leak
2026-02-22 11:07 ` Andy Shevchenko
@ 2026-02-22 16:30 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-02-22 16:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ethan Tidmore, linusw, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Sun, 22 Feb 2026 13:07:26 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-02-22 16:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-20 20:05 [PATCH] iio: gyro: mpu3050: Fix resource leak Ethan Tidmore
2026-02-22 11:07 ` Andy Shevchenko
2026-02-22 16:30 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®