[PATCH] iio: accel: bmc150: Fix irq assumption regression

[PATCH] iio: accel: bmc150: Fix irq assumption regression

[Linus Walleij]

The code in bmc150-accel-core.c unconditionally calls
bmc150_accel_set_interrupt() in the iio_buffer_setup_ops,
such as on the runtime PM resume path giving a kernel
splat like this if the device has no interrupts:

Unable to handle kernel NULL pointer dereference at virtual
  address 00000001 when read
CPU: 0 UID: 0 PID: 393 Comm: iio-sensor-prox Not tainted
  6.18.0-rc1-postmarketos-stericsson-00001-g6b43386e3737 #73 PREEMPT
Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
PC is at bmc150_accel_set_interrupt+0x98/0x194
LR is at __pm_runtime_resume+0x5c/0x64
(...)
Call trace:
bmc150_accel_set_interrupt from bmc150_accel_buffer_postenable+0x40/0x108
bmc150_accel_buffer_postenable from __iio_update_buffers+0xbe0/0xcbc
__iio_update_buffers from enable_store+0x84/0xc8
enable_store from kernfs_fop_write_iter+0x154/0x1b4
kernfs_fop_write_iter from do_iter_readv_writev+0x178/0x1e4
do_iter_readv_writev from vfs_writev+0x158/0x3f4
vfs_writev from do_writev+0x74/0xe4
do_writev from __sys_trace_return+0x0/0x10

This bug seems to have been in the driver since the beginning,
but it only manifests recently, I do not know why.

Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/bmc150-accel-core.c | 5 +++++
 drivers/iio/accel/bmc150-accel.h      | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 3c5d1560b163..f4421a3b0ef2 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -523,6 +523,10 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
 	const struct bmc150_accel_interrupt_info *info = intr->info;
 	int ret;
 
+	/* We do not always have an IRQ */
+	if (!data->has_irq)
+		return 0;
+
 	if (state) {
 		if (atomic_inc_return(&intr->users) > 1)
 			return 0;
@@ -1696,6 +1700,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
 	}
 
 	if (irq > 0) {
+		data->has_irq = true;
 		ret = devm_request_threaded_irq(dev, irq,
 						bmc150_accel_irq_handler,
 						bmc150_accel_irq_thread_handler,
diff --git a/drivers/iio/accel/bmc150-accel.h b/drivers/iio/accel/bmc150-accel.h
index 7a7baf52e595..6e9bb69a1fd4 100644
--- a/drivers/iio/accel/bmc150-accel.h
+++ b/drivers/iio/accel/bmc150-accel.h
@@ -59,6 +59,7 @@ enum bmc150_accel_trigger_id {
 struct bmc150_accel_data {
 	struct regmap *regmap;
 	struct regulator_bulk_data regulators[2];
+	bool has_irq;
 	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
 	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
 	struct mutex mutex;

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251027-fix-bmc150-7e568122b265

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

Re: [PATCH] iio: accel: bmc150: Fix irq assumption regression

[Andy Shevchenko]

On Mon, Oct 27, 2025 at 11:18:17AM +0100, Linus Walleij wrote:

Hmm... Isn't this already being discussed somewhere? I have some déjà-vu.

> The code in bmc150-accel-core.c unconditionally calls
> bmc150_accel_set_interrupt() in the iio_buffer_setup_ops,
> such as on the runtime PM resume path giving a kernel
> splat like this if the device has no interrupts:
> 
> Unable to handle kernel NULL pointer dereference at virtual
>   address 00000001 when read
> CPU: 0 UID: 0 PID: 393 Comm: iio-sensor-prox Not tainted
>   6.18.0-rc1-postmarketos-stericsson-00001-g6b43386e3737 #73 PREEMPT
> Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
> PC is at bmc150_accel_set_interrupt+0x98/0x194
> LR is at __pm_runtime_resume+0x5c/0x64
> (...)
> Call trace:
> bmc150_accel_set_interrupt from bmc150_accel_buffer_postenable+0x40/0x108
> bmc150_accel_buffer_postenable from __iio_update_buffers+0xbe0/0xcbc
> __iio_update_buffers from enable_store+0x84/0xc8

> enable_store from kernfs_fop_write_iter+0x154/0x1b4
> kernfs_fop_write_iter from do_iter_readv_writev+0x178/0x1e4
> do_iter_readv_writev from vfs_writev+0x158/0x3f4
> vfs_writev from do_writev+0x74/0xe4
> do_writev from __sys_trace_return+0x0/0x10

I believe the above 5 lines are not needed.

> This bug seems to have been in the driver since the beginning,
> but it only manifests recently, I do not know why.

...

> +	/* We do not always have an IRQ */
> +	if (!data->has_irq)

Wouldn't check for 0 be enough?

	if (!data->irq)

> +		return 0;

This will require to store just an irq (and perhaps we may read of local
variable in ->probe(), who knows?). But size wise it's the same as current
standalone boolean.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: bmc150: Fix irq assumption regression

[Linus Walleij]

On Mon, Oct 27, 2025 at 12:55 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> On Mon, Oct 27, 2025 at 11:18:17AM +0100, Linus Walleij wrote:

> Hmm... Isn't this already being discussed somewhere? I have some déjà-vu.

I brought it up on the PostmarkeOS IRC, otherwise I don't
know.

> > +     /* We do not always have an IRQ */
> > +     if (!data->has_irq)
>
> Wouldn't check for 0 be enough?
>
>         if (!data->irq)

But this driver does not store the IRQ number in the
instance struct because it isn't used outside of the probe()
function.

The information that needs to be stored is bool so that's
why I stored a bool.

Yours,
Linus Walleij

Re: [PATCH] iio: accel: bmc150: Fix irq assumption regression

[Andy Shevchenko]

On Mon, Oct 27, 2025 at 06:24:25PM +0100, Linus Walleij wrote:
> On Mon, Oct 27, 2025 at 12:55 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Mon, Oct 27, 2025 at 11:18:17AM +0100, Linus Walleij wrote:
> 
> > Hmm... Isn't this already being discussed somewhere? I have some déjà-vu.
> 
> I brought it up on the PostmarkeOS IRC, otherwise I don't
> know.

It might be that it was a cross-mail that describes the same issue in another
IIO driver.

> > > +     /* We do not always have an IRQ */
> > > +     if (!data->has_irq)
> >
> > Wouldn't check for 0 be enough?
> >
> >         if (!data->irq)
> 
> But this driver does not store the IRQ number in the
> instance struct because it isn't used outside of the probe()
> function.
> 
> The information that needs to be stored is bool so that's
> why I stored a bool.

I understand this, but I think storing the IRQ number is tiny better
as we might have a chance to need it in the future.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: accel: bmc150: Fix irq assumption regression

[Linus Walleij]

On Tue, Oct 28, 2025 at 12:34 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> On Mon, Oct 27, 2025 at 06:24:25PM +0100, Linus Walleij wrote:
> > On Mon, Oct 27, 2025 at 12:55 PM Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > > On Mon, Oct 27, 2025 at 11:18:17AM +0100, Linus Walleij wrote:
> >
> > > Hmm... Isn't this already being discussed somewhere? I have some déjà-vu.
> >
> > I brought it up on the PostmarkeOS IRC, otherwise I don't
> > know.
>
> It might be that it was a cross-mail that describes the same issue in another
> IIO driver.

Hm, I'm a bit worried that this may be a generic problem
in several drivers that now think they can be used without
IRQ but actually crash if you try to do that :/

> > > Wouldn't check for 0 be enough?
> > >
> > >         if (!data->irq)
> >
> > But this driver does not store the IRQ number in the
> > instance struct because it isn't used outside of the probe()
> > function.
> >
> > The information that needs to be stored is bool so that's
> > why I stored a bool.
>
> I understand this, but I think storing the IRQ number is tiny better
> as we might have a chance to need it in the future.

Fair enough, it's a common pattern so I'll rewrite the patch
like this!

Yours,
Linus Walleij