On Mon, 27 Mar 2023 22:02:48 +0200 Fabrizio Lamarque <fl.scratchpad@xxxxxxxxx> wrote: > Fix ad7192.c NULL pointer dereference in ad7192_setup() when accessing > indio_dev structure while populating input rages, causing a kernel > panic. > Fixed by calling spi_set_drvdata after indio_dev is allocated. > > Additional details > > Kernel panic log > [ 5.763067] Unable to handle kernel NULL pointer dereference at > virtual address 00000208 > [...] > [ 6.265076] [<c063b94c>] (driver_register) from [<c070e59c>] > (__spi_register_driver+0xd8/0xe4) > [ 6.273757] r5:c0b88c7c r4:00000000 > [ 6.277351] [<c070e4c4>] (__spi_register_driver) from [<c0e4c288>] > (ad7192_driver_init+0x20/0x28) > [ 6.286305] r9:c107fe00 r8:c107fe00 r7:00000000 r6:c0e56854 > r5:c0e4c268 r4:c40fa000 > [ 6.294070] [<c0e4c268>] (ad7192_driver_init) from [<c01023d0>] > (do_one_initcall+0x58/0x2ac) > [ 6.302569] [<c0102378>] (do_one_initcall) from [<c0e01594>] > (kernel_init_freeable+0x1c4/0x254) > [...] > [ 6.387349] Kernel panic - not syncing: Attempted to kill init! > exitcode=0x0000000b > [ 6.395049] ---[ end Kernel panic - not syncing: Attempted to kill > init! exitcode=0x0000000b ] > > The patch is against the current tree, but it applies without > modifications to 5.x (the driver has not changed much since then). > Reproduced in kernel version 5.15.x. Newer driver versions are > affected by the same issue. > > Pointer to indio_dev structure is obtained via spi_get_drvdata() at > the beginning of function ad7192_setup(), but the > spi->dev->driver_data member is not initialized here, hence a NULL > pointer is returned. > > By comparing every other iio adc driver, whenever there is a call to > spi_get_drvdata() there is also one to spi_set_drvdata() within probe > function. > It should also be noted that the indio_dev structure is accessed just > to get the number of bits for the converter, and no other driver calls > spi_get_drvdata within probe. > After the patch is applied the system boots correctly and the ADC is > mapped within sysfs. I'd prefer to fix this by changing the ad7192_setup() to take the struct iio_dev (available at it's call site) and avoid the dance that is currently going on entirely. Drop the struct ad7192_state *st parameter and get that via st = iio_priv(indio_dev); Thanks, Jonathan > > Signed-off-by: Fabrizio Lamarque <fl.scratchpad@xxxxxxxxx> > --- linux/drivers/iio/adc/ad7192.c 2023-03-13 19:32:42.646239506 +0100 > +++ linux/drivers/iio/adc/ad7192.c 2023-03-13 19:33:41.654803797 +0100 > @@ -997,7 +997,7 @@ static int ad7192_probe(struct spi_devic > return -ENOMEM; > > st = iio_priv(indio_dev); > - > + spi_set_drvdata(spi, indio_dev); > mutex_init(&st->lock); > > st->avdd = devm_regulator_get(&spi->dev, "avdd");
