[PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[Joshua Crofts]

The mpu3050_trigger_probe() function registers a trigger but
mpu3050_common_remove() and error paths in mpu3050_common_probe() forget
to clean it up, causing a use-after-free bug.

Add the missing iio_trigger_unregister() function. Additionally, change
the mpu3050_trigger_probe() function to assign trig only after it is
registered successfully to prevent a null pointer dereference using a
local struct iio_trigger.

Reported-by: Dingisoul <dingiso.kernel@gmail.com>
Closes: https://lore.kernel.org/all/20261010203601.1686148-1-dingiso.kernel@gmail.com/
Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Cc: stable@vger.kernel.org
Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
Changes in v2:
- Add a local struct iio_trigger and assign it to the data struct only
  after a successful IRQ request to prevent a null pointer deref as
  pointed out by Sashiko
- Link to v1: https://lore.kernel.org/r/20261011-mpu3050-fix-v1-1-f55b0d138be4@gmail.com
---
 drivers/iio/gyro/mpu3050-core.c | 45 ++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 07c0f9011c63..a887b65c0d18 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -1051,14 +1051,14 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 {
 	struct mpu3050 *mpu3050 = iio_priv(indio_dev);
 	struct device *dev = mpu3050->dev;
+	struct iio_trigger *trig;
 	unsigned long irq_trig;
 	int ret;
 
-	mpu3050->trig = devm_iio_trigger_alloc(&indio_dev->dev,
-					       "%s-dev%d",
-					       indio_dev->name,
-					       iio_device_id(indio_dev));
-	if (!mpu3050->trig)
+	trig = devm_iio_trigger_alloc(&indio_dev->dev, "%s-dev%d",
+				      indio_dev->name,
+				      iio_device_id(indio_dev));
+	if (!trig)
 		return -ENOMEM;
 
 	/* Check if IRQ is open drain */
@@ -1115,30 +1115,29 @@ static int mpu3050_trigger_probe(struct iio_dev *indio_dev, int irq)
 				   mpu3050_irq_handler,
 				   mpu3050_irq_thread,
 				   irq_trig,
-				   mpu3050->trig->name,
-				   mpu3050->trig);
+				   trig->name,
+				   trig);
 	if (ret) {
 		dev_err(dev, "can't get IRQ %d, error %d\n", irq, ret);
 		return ret;
 	}
 
+	trig->dev.parent = dev;
+	trig->ops = &mpu3050_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = iio_trigger_register(trig);
+	if (ret) {
+		free_irq(irq, trig);
+		return ret;
+	}
+
+	mpu3050->trig = trig;
 	mpu3050->irq = irq;
-	mpu3050->trig->dev.parent = dev;
-	mpu3050->trig->ops = &mpu3050_trigger_ops;
-	iio_trigger_set_drvdata(mpu3050->trig, indio_dev);
 
-	ret = iio_trigger_register(mpu3050->trig);
-	if (ret)
-		goto err_iio_trigger;
-
-	indio_dev->trig = iio_trigger_get(mpu3050->trig);
+	indio_dev->trig = iio_trigger_get(trig);
 
 	return 0;
-
-err_iio_trigger:
-	free_irq(mpu3050->irq, mpu3050->trig);
-
-	return ret;
 }
 
 int mpu3050_common_probe(struct device *dev,
@@ -1260,7 +1259,9 @@ 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->trig)
+		iio_trigger_unregister(mpu3050->trig);
+	if (mpu3050->irq)
 		free_irq(mpu3050->irq, mpu3050->trig);
 	iio_triggered_buffer_cleanup(indio_dev);
 err_power_down:
@@ -1278,6 +1279,8 @@ void mpu3050_common_remove(struct device *dev)
 	pm_runtime_get_sync(dev);
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
+	if (mpu3050->trig)
+		iio_trigger_unregister(mpu3050->trig);
 	if (mpu3050->irq)
 		free_irq(mpu3050->irq, mpu3050->trig);
 	iio_triggered_buffer_cleanup(indio_dev);

---
base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
change-id: 20261011-mpu3050-fix-145a77d5229b

Best regards,
-- 
Joshua Crofts

Re: [PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[Joshua Crofts]

On Thu, 15 Oct 2026 21:58:35 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> The mpu3050_trigger_probe() function registers a trigger but
> mpu3050_common_remove() and error paths in mpu3050_common_probe() forget
> to clean it up, causing a use-after-free bug.
> 
> Add the missing iio_trigger_unregister() function. Additionally, change
> the mpu3050_trigger_probe() function to assign trig only after it is
> registered successfully to prevent a null pointer dereference using a
> local struct iio_trigger.
> 
> Reported-by: Dingisoul <dingiso.kernel@gmail.com>
> Closes: https://lore.kernel.org/all/20261010203601.1686148-1-dingiso.kernel@gmail.com/
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

Dingisoul, any chance you can test this?

Q to others: better to leave the driver running in this weird
polling mode as it is now or return on trigger register failure
as Nuno suggests?

Thanks.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[Dingisoul]

Hi Joshua,

v2 looks correct -- mpu3050->trig/mpu3050->irq are now only ever set
together after iio_trigger_register() succeeds, so the unregister/
free_irq guards in the error path and mpu3050_common_remove() can
never fire on a trigger or IRQ that was never actually acquired.

Applied it, rebuilt, and reran my reproducer: the trigger now gets
properly unregistered on unbind, so the attack just gets -EINVAL
instead of a UAF. No KASAN report, no warnings.

Tested-by: Dingisoul <dingiso.kernel@gmail.com>

Best,
Dingisoul

Re: [PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[Joshua Crofts]

On Thu, 15 Oct 2026 16:35:59 -0400
Dingisoul <dingiso.kernel@gmail.com> wrote:

> Applied it, rebuilt, and reran my reproducer: the trigger now gets
> properly unregistered on unbind, so the attack just gets -EINVAL
> instead of a UAF. No KASAN report, no warnings.

Good to hear, thanks for testing!

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2] iio: gyro: mpu3050: add missing trigger cleanup on remove

[Nuno Sá]

On Thu, Oct 15, 2026 at 10:18:18PM +0200, Joshua Crofts wrote:
> On Thu, 15 Oct 2026 21:58:35 +0200
> Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> 
> > The mpu3050_trigger_probe() function registers a trigger but
> > mpu3050_common_remove() and error paths in mpu3050_common_probe() forget
> > to clean it up, causing a use-after-free bug.
> > 
> > Add the missing iio_trigger_unregister() function. Additionally, change
> > the mpu3050_trigger_probe() function to assign trig only after it is
> > registered successfully to prevent a null pointer dereference using a
> > local struct iio_trigger.
> > 
> > Reported-by: Dingisoul <dingiso.kernel@gmail.com>
> > Closes: https://lore.kernel.org/all/20261010203601.1686148-1-dingiso.kernel@gmail.com/
> > Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> > ---
> 
> Dingisoul, any chance you can test this?
> 
> Q to others: better to leave the driver running in this weird
> polling mode as it is now or return on trigger register failure
> as Nuno suggests?

Don't think it makes sense to continue if the idea is to have trigger
but something failed when dealing with it. At the very least we should
then drop the memory we allocated for it.

- Nuno Sá
> 
> Thanks.
> 
> -- 
> Kind regards,
> Joshua Crofts