| Message ID | 1517482822-28758-1-git-send-email-himanshujha199640@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Himanshu, On Thu, Feb 1, 2018 at 12:00 PM, Himanshu Jha <himanshujha199640@gmail.com> wrote: > In scpsys_probe function, return value of of_match_device function which > returns null is dereferenced without checking. Therefore, add a check for > potential null dereference. > > Detected by CoverityScan, CID#1424087 "Dereference null return value" > > Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of scpsys_probe across all SoCs") > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> This is a false positive: as this is a pure-OF driver, scpsys_probe() is called if and only if a match was found in of_scpsys_match_tbl[]. > --- > drivers/soc/mediatek/mtk-scpsys.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c > index 435ce5e..6e7f196 100644 > --- a/drivers/soc/mediatek/mtk-scpsys.c > +++ b/drivers/soc/mediatek/mtk-scpsys.c > @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) > int i, ret; > > match = of_match_device(of_scpsys_match_tbl, &pdev->dev); > + if (!match) > + return -EINVAL; > + > soc = (const struct scp_soc_data *)match->data; > > scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs, Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 01/02/2018 at 16:30:22 +0530, Himanshu Jha wrote: > In scpsys_probe function, return value of of_match_device function which > returns null is dereferenced without checking. Therefore, add a check for > potential null dereference. > > Detected by CoverityScan, CID#1424087 "Dereference null return value" > No, this will never happen as the only way to probe this device is to use device tree. > Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of scpsys_probe across all SoCs") > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> > --- > drivers/soc/mediatek/mtk-scpsys.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c > index 435ce5e..6e7f196 100644 > --- a/drivers/soc/mediatek/mtk-scpsys.c > +++ b/drivers/soc/mediatek/mtk-scpsys.c > @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) > int i, ret; > > match = of_match_device(of_scpsys_match_tbl, &pdev->dev); > + if (!match) > + return -EINVAL; > + > soc = (const struct scp_soc_data *)match->data; > > scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs, > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 01/02/18 11:00, Himanshu Jha wrote: > In scpsys_probe function, return value of of_match_device function which > returns null is dereferenced without checking. Therefore, add a check for > potential null dereference. > > Detected by CoverityScan, CID#1424087 "Dereference null return value" > > Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of scpsys_probe across all SoCs") > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> > --- > drivers/soc/mediatek/mtk-scpsys.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c > index 435ce5e..6e7f196 100644 > --- a/drivers/soc/mediatek/mtk-scpsys.c > +++ b/drivers/soc/mediatek/mtk-scpsys.c > @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) > int i, ret; > > match = of_match_device(of_scpsys_match_tbl, &pdev->dev); > + if (!match) > + return -EINVAL; > + > soc = (const struct scp_soc_data *)match->data; You could of course replace the whole sequence with an of_device_get_match_data() call, which happens to be inherently safe against the no-match case even when that *is* impossible by design. Robin. > > scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs, >
On Thu, Feb 1, 2018 at 4:02 PM, Robin Murphy <robin.murphy@arm.com> wrote: > On 01/02/18 11:00, Himanshu Jha wrote: >> In scpsys_probe function, return value of of_match_device function which >> returns null is dereferenced without checking. Therefore, add a check for >> potential null dereference. >> >> Detected by CoverityScan, CID#1424087 "Dereference null return value" >> >> Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of >> scpsys_probe across all SoCs") >> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> >> --- >> drivers/soc/mediatek/mtk-scpsys.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/soc/mediatek/mtk-scpsys.c >> b/drivers/soc/mediatek/mtk-scpsys.c >> index 435ce5e..6e7f196 100644 >> --- a/drivers/soc/mediatek/mtk-scpsys.c >> +++ b/drivers/soc/mediatek/mtk-scpsys.c >> @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) >> int i, ret; >> match = of_match_device(of_scpsys_match_tbl, &pdev->dev); >> + if (!match) >> + return -EINVAL; >> + >> soc = (const struct scp_soc_data *)match->data; > > You could of course replace the whole sequence with an > of_device_get_match_data() call, which happens to be inherently safe against > the no-match case even when that *is* impossible by design. +1 >> scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs, ... followed by the static analyser gang complaining we may dereference NULL pointer soc... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 01/02/18 15:09, Geert Uytterhoeven wrote: > On Thu, Feb 1, 2018 at 4:02 PM, Robin Murphy <robin.murphy@arm.com> wrote: >> On 01/02/18 11:00, Himanshu Jha wrote: >>> In scpsys_probe function, return value of of_match_device function which >>> returns null is dereferenced without checking. Therefore, add a check for >>> potential null dereference. >>> >>> Detected by CoverityScan, CID#1424087 "Dereference null return value" >>> >>> Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of >>> scpsys_probe across all SoCs") >>> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> >>> --- >>> drivers/soc/mediatek/mtk-scpsys.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c >>> b/drivers/soc/mediatek/mtk-scpsys.c >>> index 435ce5e..6e7f196 100644 >>> --- a/drivers/soc/mediatek/mtk-scpsys.c >>> +++ b/drivers/soc/mediatek/mtk-scpsys.c >>> @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) >>> int i, ret; >>> match = of_match_device(of_scpsys_match_tbl, &pdev->dev); >>> + if (!match) >>> + return -EINVAL; >>> + >>> soc = (const struct scp_soc_data *)match->data; >> >> You could of course replace the whole sequence with an >> of_device_get_match_data() call, which happens to be inherently safe against >> the no-match case even when that *is* impossible by design. > > +1 > >>> scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs, > > ... followed by the static analyser gang complaining we may dereference > NULL pointer soc... Well, if the static analysers can't track the provenance of dev->driver->of_match_table, let's keep ignoring them until they get cleverer :P Robin. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c index 435ce5e..6e7f196 100644 --- a/drivers/soc/mediatek/mtk-scpsys.c +++ b/drivers/soc/mediatek/mtk-scpsys.c @@ -981,6 +981,9 @@ static int scpsys_probe(struct platform_device *pdev) int i, ret; match = of_match_device(of_scpsys_match_tbl, &pdev->dev); + if (!match) + return -EINVAL; + soc = (const struct scp_soc_data *)match->data; scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs,
In scpsys_probe function, return value of of_match_device function which returns null is dereferenced without checking. Therefore, add a check for potential null dereference. Detected by CoverityScan, CID#1424087 "Dereference null return value" Fixes: commit 53fddb1a66dd ("soc: mediatek: reduce code duplication of scpsys_probe across all SoCs") Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com> --- drivers/soc/mediatek/mtk-scpsys.c | 3 +++ 1 file changed, 3 insertions(+)