[v1] iio: gyro: mpu3050: use devm_iio_trigger_register

[PATCH] iio: gyro: mpu3050: use devm_iio_trigger_register

Posted by lixinyu 4 days, 21 hours ago

mpu3050_trigger_probe() allocates the DRDY trigger with
devm_iio_trigger_alloc() but registers it with plain
iio_trigger_register(). The remove callback calls free_irq()
on the trigger but never calls iio_trigger_unregister(), so on
module unload the trigger remains in the global trigger list
while its memory is freed by devm, leaving a dangling entry.

Switch to devm_iio_trigger_register() so the registration is
undone automatically in the same devm scope as the allocation.

Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Cc: stable@vger.kernel.org
Signed-off-by: lixinyu <xinyuili@126.com>
---
 drivers/iio/gyro/mpu3050-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..bcfa83a46737 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1127,7 +1127,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 	mpu3050->trig->ops = &mpu3050_trigger_ops;
 	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
 
-	ret = iio_trigger_register(mpu3050->trig);
+	ret = devm_iio_trigger_register(mpu3050->dev, mpu3050->trig);
 	if (ret)
 		goto err_iio_trigger;
 
-- 
2.34.1

Re: [PATCH] iio: gyro: mpu3050: use devm_iio_trigger_register

Posted by Maxwell Doose 4 days, 21 hours ago

On Tue, May 19, 2026 at 9:43 PM lixinyu <xinyuili@126.com> wrote:
>
> mpu3050_trigger_probe() allocates the DRDY trigger with
> devm_iio_trigger_alloc() but registers it with plain
> iio_trigger_register(). The remove callback calls free_irq()
> on the trigger but never calls iio_trigger_unregister(), so on
> module unload the trigger remains in the global trigger list
> while its memory is freed by devm, leaving a dangling entry.
>
> Switch to devm_iio_trigger_register() so the registration is
> undone automatically in the same devm scope as the allocation.
>
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
> Signed-off-by: lixinyu <xinyuili@126.com>

Hm...this signed-off-by doesn't look right (unless, of course,
"lixinyu" is your real name and not "Li Xinyu" or something like
that). Please correct if you send v2.

best regards,
max

[PATCH v3] iio: gyro: mpu3050: fix missing iio_trigger_unregister and irq cleanup

Posted by Li Xinyu 4 days, 9 hours ago

mpu3050_trigger_probe() registers the DRDY trigger with
iio_trigger_register() but neither mpu3050_common_remove() nor
the error path in mpu3050_common_probe() calls
iio_trigger_unregister(). On module unload or probe failure the
trigger remains in the global trigger list while its memory is
freed by devm, leaving a dangling entry.

Also fix a use-after-free risk: when iio_trigger_register() fails,
mpu3050->irq remained set to a non-zero value, which would cause
mpu3050_common_remove() to attempt a double-free of the IRQ and
an unregister of a never-registered trigger. Clear mpu3050->irq
in the error path to prevent this.

Revert the v2 devm approach as requested by Jonathan: the driver
mixes devm and non-devm resource management, so the minimal fix
is to add the missing unregister calls and keep the existing
manual resource management style.

Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Cc: stable@vger.kernel.org
Signed-off-by: Li Xinyu <xinyuili@126.com>
---
Changes in v3:
- Thanks Jonathan for the feedback on v2. Instead of mixing devm
  with non-devm resource management in probe, revert to plain
  iio_trigger_register() and add the missing iio_trigger_unregister()
  calls in the error path and remove callback.
- Also noticed that mpu3050->irq was set but not cleared when
  iio_trigger_register() fails in trigger_probe, which would
  cause a double-free on module unload. Set mpu3050->irq = 0
  in the error path to prevent this.

Changes in v2:
- Fixed the name format in Signed-off-by. Thanks Maxime for
  catching this.
---
 drivers/iio/gyro/mpu3050-core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index bcfa83a46737..459d02aa3d18 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1127,7 +1127,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 	mpu3050->trig->ops = &mpu3050_trigger_ops;
 	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
 
-	ret = devm_iio_trigger_register(mpu3050->dev, mpu3050->trig);
+	ret = iio_trigger_register(mpu3050->trig);
 	if (ret)
 		goto err_iio_trigger;
 
@@ -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,8 +1261,10 @@ 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) {
+		iio_trigger_unregister(mpu3050->trig);
 		free_irq(mpu3050->irq, mpu3050->trig);
+	}
 	iio_triggered_buffer_cleanup(indio_dev);
 err_power_down:
 	mpu3050_power_down(mpu3050);
@@ -1278,8 +1281,10 @@ void mpu3050_common_remove(struct device *dev)
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
-	if (mpu3050->irq)
+	if (mpu3050->irq) {
+		iio_trigger_unregister(mpu3050->trig);
 		free_irq(mpu3050->irq, mpu3050->trig);
+	}
 	iio_triggered_buffer_cleanup(indio_dev);
 	mpu3050_power_down(mpu3050);
 }
-- 
2.34.1

Re: [PATCH v3] iio: gyro: mpu3050: fix missing iio_trigger_unregister and irq cleanup

Posted by Jonathan Cameron 3 days, 12 hours ago

On Wed, 20 May 2026 23:22:36 +0800
Li Xinyu <xinyuili@126.com> wrote:

> mpu3050_trigger_probe() registers the DRDY trigger with
> iio_trigger_register() but neither mpu3050_common_remove() nor
> the error path in mpu3050_common_probe() calls
> iio_trigger_unregister(). On module unload or probe failure the
> trigger remains in the global trigger list while its memory is
> freed by devm, leaving a dangling entry.
> 
> Also fix a use-after-free risk: when iio_trigger_register() fails,
> mpu3050->irq remained set to a non-zero value, which would cause
> mpu3050_common_remove() to attempt a double-free of the IRQ and
> an unregister of a never-registered trigger. Clear mpu3050->irq
> in the error path to prevent this.
> 
> Revert the v2 devm approach as requested by Jonathan: the driver
> mixes devm and non-devm resource management, so the minimal fix
> is to add the missing unregister calls and keep the existing
> manual resource management style.
> 
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Xinyu <xinyuili@126.com>
> ---
> Changes in v3:
> - Thanks Jonathan for the feedback on v2. Instead of mixing devm
>   with non-devm resource management in probe, revert to plain
>   iio_trigger_register() and add the missing iio_trigger_unregister()
>   calls in the error path and remove callback.
> - Also noticed that mpu3050->irq was set but not cleared when
>   iio_trigger_register() fails in trigger_probe, which would
>   cause a double-free on module unload. Set mpu3050->irq = 0
>   in the error path to prevent this.

This is interesting. I wonder why we paper over the failed trigger
registration.   Generally that's an error case that should
result in the driver not loading.

The mix of devm and non devm in iio_trigger_register() is also
nasty.

Linus W, I think this was your code, any idea if we actually need
to do that and can't just fail?  If we can modify it to fail
I'd rather do that and avoid using ->irq as a flag to indicate
if we successfully registered or not

> 
> Changes in v2:
> - Fixed the name format in Signed-off-by. Thanks Maxime for
>   catching this.
> ---
>  drivers/iio/gyro/mpu3050-core.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index bcfa83a46737..459d02aa3d18 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1127,7 +1127,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
>  	mpu3050->trig->ops = &mpu3050_trigger_ops;
>  	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
>  
> -	ret = devm_iio_trigger_register(mpu3050->dev, mpu3050->trig);
> +	ret = iio_trigger_register(mpu3050->trig);
>  	if (ret)
>  		goto err_iio_trigger;
>  
> @@ -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,8 +1261,10 @@ 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) {
> +		iio_trigger_unregister(mpu3050->trig);
>  		free_irq(mpu3050->irq, mpu3050->trig);
> +	}
>  	iio_triggered_buffer_cleanup(indio_dev);
>  err_power_down:
>  	mpu3050_power_down(mpu3050);
> @@ -1278,8 +1281,10 @@ void mpu3050_common_remove(struct device *dev)
>  	pm_runtime_get_sync(dev);
>  	pm_runtime_put_noidle(dev);
>  	pm_runtime_disable(dev);
> -	if (mpu3050->irq)
> +	if (mpu3050->irq) {
> +		iio_trigger_unregister(mpu3050->trig);
>  		free_irq(mpu3050->irq, mpu3050->trig);
> +	}
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	mpu3050_power_down(mpu3050);
>  }

[PATCH v2] iio: gyro: mpu3050: use devm_iio_trigger_register

Posted by Li Xinyu 4 days, 20 hours ago

mpu3050_trigger_probe() allocates the DRDY trigger with
devm_iio_trigger_alloc() but registers it with plain
iio_trigger_register(). The remove callback calls free_irq()
on the trigger but never calls iio_trigger_unregister(), so on
module unload the trigger remains in the global trigger list
while its memory is freed by devm, leaving a dangling entry.

Switch to devm_iio_trigger_register() so the registration is
undone automatically in the same devm scope as the allocation.

Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Cc: stable@vger.kernel.org
Signed-off-by: Li Xinyu <xinyuili@126.com>
---
Changes in v2:
- Corrected the name format in Signed-off-by from "lixinyu" to proper
  "Li Xinyu". Sorry for the mistake in v1. Thank you Maxime.
---
 drivers/iio/gyro/mpu3050-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..bcfa83a46737 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1127,7 +1127,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 	mpu3050->trig->ops = &mpu3050_trigger_ops;
 	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
 
-	ret = iio_trigger_register(mpu3050->trig);
+	ret = devm_iio_trigger_register(mpu3050->dev, mpu3050->trig);
 	if (ret)
 		goto err_iio_trigger;
 
-- 
2.34.1

Re: [PATCH v2] iio: gyro: mpu3050: use devm_iio_trigger_register

Posted by Jonathan Cameron 4 days, 13 hours ago

On Wed, 20 May 2026 11:24:47 +0800
Li Xinyu <xinyuili@126.com> wrote:

> mpu3050_trigger_probe() allocates the DRDY trigger with
> devm_iio_trigger_alloc() but registers it with plain
> iio_trigger_register(). The remove callback calls free_irq()
> on the trigger but never calls iio_trigger_unregister(), so on
> module unload the trigger remains in the global trigger list
> while its memory is freed by devm, leaving a dangling entry.
> 
> Switch to devm_iio_trigger_register() so the registration is
> undone automatically in the same devm scope as the allocation.
> 
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Xinyu <xinyuili@126.com>
Look at the more general use of devm in this driver.

The rule of thumb for devm is that you can only use it from start of
probe() to the point where you first make a call that doesn't use it.
After that you must not use any devm calls.

Sometimes it is easy to use it for all of probe() which obviously obeys
that rule.

Also when you do switch to devm in a driver, there is normally a
reverse operation to remove in remove() and error paths().

There was a recent fix for a case similar to this but that was because
that driver did everything else with devm and didn't call the unwind
at all. It was a simple typo in the driver.

Jonathan


> ---
> Changes in v2:
> - Corrected the name format in Signed-off-by from "lixinyu" to proper
>   "Li Xinyu". Sorry for the mistake in v1. Thank you Maxime.
> ---
>  drivers/iio/gyro/mpu3050-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b431..bcfa83a46737 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1127,7 +1127,7 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
>  	mpu3050->trig->ops = &mpu3050_trigger_ops;
>  	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
>  
> -	ret = iio_trigger_register(mpu3050->trig);
> +	ret = devm_iio_trigger_register(mpu3050->dev, mpu3050->trig);
>  	if (ret)
>  		goto err_iio_trigger;
>