of_match_device() may fail and returns a NULL pointer.
Fix this by checking the return value of of_match_device().
Fixes: 64ad7f6438f3 ("iio: adc: stm32: introduce compatible data cfg")
Signed-off-by: Zhang Shurong <[email protected]>
---
drivers/iio/adc/stm32-adc-core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index 48f02dcc81c1..70011fdbf5f6 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -706,6 +706,8 @@ static int stm32_adc_probe(struct platform_device *pdev)
struct stm32_adc_priv *priv;
struct device *dev = &pdev->dev;
struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *of_id;
+
struct resource *res;
u32 max_rate;
int ret;
@@ -718,8 +720,11 @@ static int stm32_adc_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, &priv->common);
- priv->cfg = (const struct stm32_adc_priv_cfg *)
- of_match_device(dev->driver->of_match_table, dev)->data;
+ of_id = of_match_device(dev->driver->of_match_table, dev);
+ if (!of_id)
+ return -ENODEV;
+
+ priv->cfg = (const struct stm32_adc_priv_cfg *)of_id->data;
priv->nb_adc_max = priv->cfg->num_adcs;
spin_lock_init(&priv->common.lock);
--
2.30.2
On Sat, 15 Jul 2023 23:55:50 +0800
Zhang Shurong <[email protected]> wrote:
> of_match_device() may fail and returns a NULL pointer.
>
> Fix this by checking the return value of of_match_device().
>
> Fixes: 64ad7f6438f3 ("iio: adc: stm32: introduce compatible data cfg")
> Signed-off-by: Zhang Shurong <[email protected]>
Hi Zhang,
I'm not sure we can actually make this bug happen. Do you have
a way of triggering it? The driver is only probed on devices where
that match will work.
Also, assuming the match table is the same one associated with this probe
function, then us priv->cfg = of_device_get_match_data() and check the output
of that which is what we really care about.
Jonathan
> ---
> drivers/iio/adc/stm32-adc-core.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> index 48f02dcc81c1..70011fdbf5f6 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -706,6 +706,8 @@ static int stm32_adc_probe(struct platform_device *pdev)
> struct stm32_adc_priv *priv;
> struct device *dev = &pdev->dev;
> struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *of_id;
> +
> struct resource *res;
> u32 max_rate;
> int ret;
> @@ -718,8 +720,11 @@ static int stm32_adc_probe(struct platform_device *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, &priv->common);
>
> - priv->cfg = (const struct stm32_adc_priv_cfg *)
> - of_match_device(dev->driver->of_match_table, dev)->data;
> + of_id = of_match_device(dev->driver->of_match_table, dev);
> + if (!of_id)
> + return -ENODEV;
> +
> + priv->cfg = (const struct stm32_adc_priv_cfg *)of_id->data;
> priv->nb_adc_max = priv->cfg->num_adcs;
> spin_lock_init(&priv->common.lock);
>
On Tue, 29 Aug 2023 00:35:36 +0800
Zhang Shurong <[email protected]> wrote:
> 在 2023年8月29日星期二 CST 上午12:16:05，Jonathan Cameron 写道：
> > On Mon, 28 Aug 2023 23:02:07 +0800
> >
> > Zhang Shurong <[email protected]> wrote:
> > > 在 2023年7月17日星期一 CST 上午12:08:21，Jonathan Cameron 写道：
> > >
> > > > On Sat, 15 Jul 2023 23:55:50 +0800
> > > >
> > > > Zhang Shurong <[email protected]> wrote:
> > > > > of_match_device() may fail and returns a NULL pointer.
> > > > >
> > > > > Fix this by checking the return value of of_match_device().
> > > > >
> > > > > Fixes: 64ad7f6438f3 ("iio: adc: stm32: introduce compatible data cfg")
> > > > > Signed-off-by: Zhang Shurong <[email protected]>
> > > >
> > > > Hi Zhang,
> > > >
> > > > I'm not sure we can actually make this bug happen. Do you have
> > > > a way of triggering it? The driver is only probed on devices where
> > > > that match will work.
> > > >
> > > > Also, assuming the match table is the same one associated with this
> > > > probe
> > > > function, then us priv->cfg = of_device_get_match_data() and check the
> > > > output of that which is what we really care about.
> > > >
> > > > Jonathan
> > > >
> > > > > ---
> > > > >
> > > > > drivers/iio/adc/stm32-adc-core.c | 9 +++++++--
> > > > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/adc/stm32-adc-core.c
> > > > > b/drivers/iio/adc/stm32-adc-core.c index 48f02dcc81c1..70011fdbf5f6
> > > > > 100644
> > > > > --- a/drivers/iio/adc/stm32-adc-core.c
> > > > > +++ b/drivers/iio/adc/stm32-adc-core.c
> > > > > @@ -706,6 +706,8 @@ static int stm32_adc_probe(struct platform_device
> > > > > *pdev)>
> > > > >
> > > > > struct stm32_adc_priv *priv;
> > > > > struct device *dev = &pdev->dev;
> > > > > struct device_node *np = pdev->dev.of_node;
> > > > >
> > > > > + const struct of_device_id *of_id;
> > > > > +
> > > > >
> > > > > struct resource *res;
> > > > > u32 max_rate;
> > > > > int ret;
> > > > >
> > > > > @@ -718,8 +720,11 @@ static int stm32_adc_probe(struct platform_device
> > > > > *pdev)>
> > > > >
> > > > > return -ENOMEM;
> > > > >
> > > > > platform_set_drvdata(pdev, &priv->common);
> > > > >
> > > > > - priv->cfg = (const struct stm32_adc_priv_cfg *)
> > > > > - of_match_device(dev->driver->of_match_table, dev)->data;
> > > > > + of_id = of_match_device(dev->driver->of_match_table, dev);
> > > > > + if (!of_id)
> > > > > + return -ENODEV;
> > > > > +
> > > > > + priv->cfg = (const struct stm32_adc_priv_cfg *)of_id->data;
> > > > >
> > > > > priv->nb_adc_max = priv->cfg->num_adcs;
> > > > > spin_lock_init(&priv->common.lock);
> > >
> > > Hello Jonathan,
> > >
> > > I think we can make it happen by designing the platform device in a way
> > > that its name aligns with that of the driver. In such a scenario, when
> > > the driver is probed, the of_match_device function will return null. You
> > > can verify this functionality by reviewing the following function:
> > >
> > > static int platform_match(struct device *dev, struct device_driver *drv)
> >
> > I don't think we care about that case. If there is a real example of
> > why someone would do this then that would be a different matter.
> >
> > Jonathan
> >
> > > Best regards,
> > > Shurong
> I just think it might be more appropriate to return the correct error code in
> this situation. I agree with your assessment that it is an abnormal case.
> Therefore, it is perfectly fine if you decide not to select this patch.
>
> Thanks for your kind reply.
>
Applied but with a modified commit message to describe this as hardening rather
than a fix. Also dropped the fixes tag as I don't really want this to be
picked up for stable.
Thanks,
Jonathan
> Shurong
>
>
>