All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joshua Crofts <joshua.crofts1@gmail.com>
To: "Linus Walleij" <linusw@kernel.org>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>
Cc: linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dingisoul <dingiso.kernel@gmail.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] iio: gyro: mpu3050: add missing trigger cleanup on remove
Date: Sun, 11 Oct 2026 19:12:40 +0200	[thread overview]
Message-ID: <20261011191240.5a2ba4f7@systembl0wer> (raw)
In-Reply-To: <20261011-mpu3050-fix-v1-1-f55b0d138be4@gmail.com>

On Sun, 11 Oct 2026 17:53:01 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
>  drivers/iio/gyro/mpu3050-core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Sashiko review came back and the issue doesn't seem so
straight-forward. I'll probably introduce a local pointer
and then assign it to the mpu3050 data struct.

Attaching Sashiko review below.

> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index 07c0f9011c63..096fd0dc608e 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1260,6 +1260,8 @@ int mpu3050_common_probe(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);


Does this unconditionally unregister a trigger that may not have been
successfully registered?

If mpu3050_trigger_probe() fails (for example, if request_threaded_irq()
fails), mpu3050->trig is allocated via devm_iio_trigger_alloc() but it will
never have successfully completed iio_trigger_register().

mpu3050_common_probe() catches that error, logs "failed to register trigger",
and continues. If a subsequent step like iio_device_register() fails, this
code will call iio_trigger_unregister() on the uninitialized trigger.

Because the trigger was never registered via iio_trigger_register(),
iio_trigger_unregister() will call device_del() on a device that was never
added, and ida_free() on an unallocated ID, which can lead to a crash.

>  	if (irq)
>  		free_irq(mpu3050->irq, mpu3050->trig);
>  	iio_triggered_buffer_cleanup(indio_dev);
> @@ -1278,6 +1280,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);

Can this cause a kernel crash during normal driver teardown if the trigger
probe originally failed?

Similar to the probe error path above, if mpu3050_trigger_probe() failed
during initialization, mpu3050->trig is left allocated but unregistered.

Calling iio_trigger_unregister() here on that uninitialized trigger will
cause the same teardown issues during driver removal.

-- 
Kind regards,
Joshua Crofts

  reply	other threads:[~2026-10-11 17:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-11 15:53 [PATCH] iio: gyro: mpu3050: add missing trigger cleanup on remove Joshua Crofts
2026-10-11 17:12 ` Joshua Crofts [this message]
2026-10-13 12:30   ` Nuno Sá

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261011191240.5a2ba4f7@systembl0wer \
    --to=joshua.crofts1@gmail.com \
    --cc=andy@kernel.org \
    --cc=dingiso.kernel@gmail.com \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.