Re: [PATCH 1/2] ad7192 driver: fix null pointer dereference in probe when populating adc input ranges

[Fabrizio Lamarque <fl.scratchpad@xxxxxxxxx>]

On Sat, Apr 1, 2023 at 4:13 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> 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.
> >
> > 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
>

Fix NULL pointer dereference in ad7192_setup() (ad7192.c) when accessing
indio_dev structure while populating input rages, causing a kernel panic.

Changed ad7192_setup() signature to take pointer to struct
iio_dev, and got ad7192_state pointer via st = iio_priv(indio_dev);

Fixes: bd5dcdeb3fd0 iio: adc: ad7192: convert to device-managed functions
Signed-off-by: Fabrizio Lamarque <fl.scratchpad@xxxxxxxxx>
---
V1 -> Revised after suggestions from Jonathan, removed Reviewed-by
 since the entire patch changed its content.

 drivers/iio/adc/ad7192.c | 6 +++---
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -380,9 +380,9 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
     return clock_sel;
 }

-static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
+static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
 {
-    struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
+    struct ad7192_state *st = iio_priv(indio_dev);
     bool rej60_en, refin2_en;
     bool buf_en, bipolar, burnout_curr_en;
     unsigned long long scale_uv;
@@ -1073,7 +1073,7 @@ static int ad7192_probe(struct spi_device *spi)
         }
     }

-    ret = ad7192_setup(st, spi->dev.of_node);
+    ret = ad7192_setup(indio_dev, spi->dev.of_node);
     if (ret)
         return ret;

-- 
2.34.1