[PATCH] iio: gyro: mpu3050: avoid double free_irq after trigger failure

[PATCH] iio: gyro: mpu3050: avoid double free_irq after trigger failure

[Guangshuo Li]

commit 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak") made
mpu3050_trigger_probe() release the IRQ when iio_trigger_register() fails
after request_threaded_irq() succeeds.

However, that failure is non-fatal to mpu3050_common_probe(). The caller
only logs the trigger setup failure and continues to register the IIO
device. Since mpu3050_trigger_probe() leaves mpu3050->irq set after
freeing the IRQ, later cleanup still believes the driver owns that IRQ.

If iio_device_register() fails, the probe error path frees the IRQ again.
If probe succeeds, mpu3050_common_remove() frees it again during unbind.

Clear mpu3050->irq after releasing the IRQ, and use mpu3050->irq rather
than the original irq argument to decide whether the probe error path
still has an IRQ to release.

Fixes: 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/iio/gyro/mpu3050-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..0fd881fdd1ad 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1137,6 +1137,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 
 err_iio_trigger:
 	free_irq(mpu3050->irq, mpu3050->trig);
+	mpu3050->irq = 0;
 
 	return ret;
 }
@@ -1260,7 +1261,7 @@ int mpu3050_common_probe(struct device *dev,
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
-	if (irq)
+	if (mpu3050->irq)
 		free_irq(mpu3050->irq, mpu3050->trig);
 	iio_triggered_buffer_cleanup(indio_dev);
 err_power_down:
-- 
2.43.0

Re: [PATCH] iio: gyro: mpu3050: avoid double free_irq after trigger failure

[Jonathan Cameron]

On Wed,  3 Jun 2026 22:16:33 +0800
Guangshuo Li <lgs201920130244@gmail.com> wrote:

> commit 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak") made
> mpu3050_trigger_probe() release the IRQ when iio_trigger_register() fails
> after request_threaded_irq() succeeds.
> 
> However, that failure is non-fatal to mpu3050_common_probe(). The caller
> only logs the trigger setup failure and continues to register the IIO
> device. Since mpu3050_trigger_probe() leaves mpu3050->irq set after
> freeing the IRQ, later cleanup still believes the driver owns that IRQ.
> 
> If iio_device_register() fails, the probe error path frees the IRQ again.
> If probe succeeds, mpu3050_common_remove() frees it again during unbind.
> 
> Clear mpu3050->irq after releasing the IRQ, and use mpu3050->irq rather
> than the original irq argument to decide whether the probe error path
> still has an IRQ to release.
> 
> Fixes: 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Please check the list for similar fixes.
https://lore.kernel.org/all/20260520152236.2308686-1-xinyuili@126.com/
has a discussion of the right solution here being to just fail the probe
if the trigger registration fails.

The current attempt to paper over that is nasty and not something
we normally do.

Jonathan

> ---
>  drivers/iio/gyro/mpu3050-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b431..0fd881fdd1ad 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1137,6 +1137,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
>  
>  err_iio_trigger:
>  	free_irq(mpu3050->irq, mpu3050->trig);
> +	mpu3050->irq = 0;
>  
>  	return ret;
>  }
> @@ -1260,7 +1261,7 @@ int mpu3050_common_probe(struct device *dev,
>  	pm_runtime_get_sync(dev);
>  	pm_runtime_put_noidle(dev);
>  	pm_runtime_disable(dev);
> -	if (irq)
> +	if (mpu3050->irq)
>  		free_irq(mpu3050->irq, mpu3050->trig);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  err_power_down:

Re: [PATCH] iio: gyro: mpu3050: avoid double free_irq after trigger failure

[Guangshuo Li]

Hi Jonathan,

Thank you for pointing this out.

On Thu, 4 Jun 2026 at 01:18, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed,  3 Jun 2026 22:16:33 +0800
> Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> > commit 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak") made
> > mpu3050_trigger_probe() release the IRQ when iio_trigger_register() fails
> > after request_threaded_irq() succeeds.
> >
> > However, that failure is non-fatal to mpu3050_common_probe(). The caller
> > only logs the trigger setup failure and continues to register the IIO
> > device. Since mpu3050_trigger_probe() leaves mpu3050->irq set after
> > freeing the IRQ, later cleanup still believes the driver owns that IRQ.
> >
> > If iio_device_register() fails, the probe error path frees the IRQ again.
> > If probe succeeds, mpu3050_common_remove() frees it again during unbind.
> >
> > Clear mpu3050->irq after releasing the IRQ, and use mpu3050->irq rather
> > than the original irq argument to decide whether the probe error path
> > still has an IRQ to release.
> >
> > Fixes: 4216db1043a3b ("iio: gyro: mpu3050: Fix irq resource leak")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> Please check the list for similar fixes.
> https://lore.kernel.org/all/20260520152236.2308686-1-xinyuili@126.com/
> has a discussion of the right solution here being to just fail the probe
> if the trigger registration fails.
>
> The current attempt to paper over that is nasty and not something
> we normally do.
>
> Jonathan
>
> > ---
> >  drivers/iio/gyro/mpu3050-core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> > index d84e04e4b431..0fd881fdd1ad 100644
> > --- a/drivers/iio/gyro/mpu3050-core.c
> > +++ b/drivers/iio/gyro/mpu3050-core.c
> > @@ -1137,6 +1137,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
> >
> >  err_iio_trigger:
> >       free_irq(mpu3050->irq, mpu3050->trig);
> > +     mpu3050->irq = 0;
> >
> >       return ret;
> >  }
> > @@ -1260,7 +1261,7 @@ int mpu3050_common_probe(struct device *dev,
> >       pm_runtime_get_sync(dev);
> >       pm_runtime_put_noidle(dev);
> >       pm_runtime_disable(dev);
> > -     if (irq)
> > +     if (mpu3050->irq)
> >               free_irq(mpu3050->irq, mpu3050->trig);
> >       iio_triggered_buffer_cleanup(indio_dev);
> >  err_power_down:
>

I missed that previous discussion. The patch you mentioned already fixes
the issue I was trying to address.

Thanks,
Guangshuo