2021-02-08 23:09:47

by Petr Štetiar

[permalink] [raw]
Subject: [PATCH] iio: chemical: scd30: fix Oops due to missing parent device

My machine Oopsed while testing SCD30 sensor in interrupt driven mode:

Unable to handle kernel NULL pointer dereference at virtual address 00000188
pgd = (ptrval)
[00000188] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.96+ #473
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
PC is at _raw_spin_lock_irqsave+0x10/0x4c
LR is at devres_add+0x18/0x38
...
[<8070ecac>] (_raw_spin_lock_irqsave) from [<804916a8>] (devres_add+0x18/0x38)
[<804916a8>] (devres_add) from [<805ef708>] (devm_iio_trigger_alloc+0x5c/0x7c)
[<805ef708>] (devm_iio_trigger_alloc) from [<805f0a90>] (scd30_probe+0x1d4/0x3f0)
[<805f0a90>] (scd30_probe) from [<805f10fc>] (scd30_i2c_probe+0x54/0x64)
[<805f10fc>] (scd30_i2c_probe) from [<80583390>] (i2c_device_probe+0x150/0x278)
[<80583390>] (i2c_device_probe) from [<8048e6c0>] (really_probe+0x1f8/0x360)

I've found out, that it's due to missing parent/owner device in iio_dev struct
which then leads to NULL pointer dereference during spinlock while registering
the device resource via devres_add().

Cc: <[email protected]> # v5.9+
Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
Signed-off-by: Petr Štetiar <[email protected]>
---
drivers/iio/chemical/scd30_core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
index 4d0d798c7cd3..33aa6eb1963d 100644
--- a/drivers/iio/chemical/scd30_core.c
+++ b/drivers/iio/chemical/scd30_core.c
@@ -697,6 +697,7 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,

dev_set_drvdata(dev, indio_dev);

+ indio_dev->dev.parent = dev;
indio_dev->info = &scd30_info;
indio_dev->name = name;
indio_dev->channels = scd30_channels;


2021-02-12 19:16:28

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] iio: chemical: scd30: fix Oops due to missing parent device

On Mon, 8 Feb 2021 23:39:47 +0100
Petr Štetiar <[email protected]> wrote:

> My machine Oopsed while testing SCD30 sensor in interrupt driven mode:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000188
> pgd = (ptrval)
> [00000188] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.96+ #473
> Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> PC is at _raw_spin_lock_irqsave+0x10/0x4c
> LR is at devres_add+0x18/0x38
> ...
> [<8070ecac>] (_raw_spin_lock_irqsave) from [<804916a8>] (devres_add+0x18/0x38)
> [<804916a8>] (devres_add) from [<805ef708>] (devm_iio_trigger_alloc+0x5c/0x7c)
> [<805ef708>] (devm_iio_trigger_alloc) from [<805f0a90>] (scd30_probe+0x1d4/0x3f0)
> [<805f0a90>] (scd30_probe) from [<805f10fc>] (scd30_i2c_probe+0x54/0x64)
> [<805f10fc>] (scd30_i2c_probe) from [<80583390>] (i2c_device_probe+0x150/0x278)
> [<80583390>] (i2c_device_probe) from [<8048e6c0>] (really_probe+0x1f8/0x360)
>
> I've found out, that it's due to missing parent/owner device in iio_dev struct
> which then leads to NULL pointer dereference during spinlock while registering
> the device resource via devres_add().
>
> Cc: <[email protected]> # v5.9+
> Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
> Signed-off-by: Petr Štetiar <[email protected]>

Hi Petr,

So, we moved this into the core a while back (to avoid exactly this sort of issue).
That change predates this introduction of this driver as it went in
in v5.8

So I think you've hit an issue with a backport here to an earlier kernel?

Jonathan


> ---
> drivers/iio/chemical/scd30_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> index 4d0d798c7cd3..33aa6eb1963d 100644
> --- a/drivers/iio/chemical/scd30_core.c
> +++ b/drivers/iio/chemical/scd30_core.c
> @@ -697,6 +697,7 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
>
> dev_set_drvdata(dev, indio_dev);
>
> + indio_dev->dev.parent = dev;
> indio_dev->info = &scd30_info;
> indio_dev->name = name;
> indio_dev->channels = scd30_channels;

2021-02-19 14:54:29

by Petr Štetiar

[permalink] [raw]
Subject: Re: [PATCH] iio: chemical: scd30: fix Oops due to missing parent device

Jonathan Cameron <[email protected]> [2021-02-12 19:12:19]:

Hi Jonathan,

> > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.96+ #473
>
> So, we moved this into the core a while back (to avoid exactly this sort of issue).
> That change predates this introduction of this driver as it went in
> in v5.8

sorry for the noise, I've missed that commit 8525df47b3d1 ("iio: core:
fix/re-introduce back parent assignment"), thank you for the hint.

> So I think you've hit an issue with a backport here to an earlier kernel?

Indeed, I've backported it to 5.4.96 as you can see in the dmesg output above.
I'll try to reproduce it again on 5.10 in the upcoming days.

Cheers,

Petr