diff mbox series

iio: chemical: scd30: fix Oops due to missing parent device

Message ID 20210208223947.32344-1-ynezz@true.cz (mailing list archive)
State New, archived
Headers show
Series iio: chemical: scd30: fix Oops due to missing parent device | expand

Commit Message

Petr Štetiar Feb. 8, 2021, 10:39 p.m. UTC
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(+)

Comments

Jonathan Cameron Feb. 12, 2021, 7:12 p.m. UTC | #1
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;
Petr Štetiar Feb. 19, 2021, 2:49 p.m. UTC | #2
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
diff mbox series

Patch

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;