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

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

[Petr Štetiar]

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: <stable@vger.kernel.org> # v5.9+
Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 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;

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

[Jonathan Cameron]

On Mon,  8 Feb 2021 23:39:47 +0100
Petr Štetiar <ynezz@true.cz> 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: <stable@vger.kernel.org> # v5.9+
> Fixes: 64b3d8b1b0f5 ("iio: chemical: scd30: add core driver")
> Signed-off-by: Petr Štetiar <ynezz@true.cz>

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;

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

[Petr Štetiar]

Jonathan Cameron <jic23@kernel.org> [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