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,
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.
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
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.
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
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Linus Walleij