* [PATCH] soc: mediatek: Handle return of of_match_device function
2018-02-01 11:00 [PATCH] soc: mediatek: Handle return of of_match_device function Himanshu Jha
@ 2018-02-01 11:04 ` Geert Uytterhoeven
2018-02-01 11:59 ` Alexandre Belloni
2018-02-01 15:02 ` Robin Murphy
2 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-02-01 11:04 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] soc: mediatek: Handle return of of_match_device function
2018-02-01 11:00 [PATCH] soc: mediatek: Handle return of of_match_device function Himanshu Jha
2018-02-01 11:04 ` Geert Uytterhoeven
@ 2018-02-01 11:59 ` Alexandre Belloni
2018-02-01 15:02 ` Robin Murphy
2 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2018-02-01 11:59 UTC (permalink / raw)
To: linux-arm-kernel
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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] soc: mediatek: Handle return of of_match_device function
2018-02-01 11:00 [PATCH] soc: mediatek: Handle return of of_match_device function Himanshu Jha
2018-02-01 11:04 ` Geert Uytterhoeven
2018-02-01 11:59 ` Alexandre Belloni
@ 2018-02-01 15:02 ` Robin Murphy
2018-02-01 15:09 ` Geert Uytterhoeven
2 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2018-02-01 15:02 UTC (permalink / raw)
To: 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,
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] soc: mediatek: Handle return of of_match_device function
2018-02-01 15:02 ` Robin Murphy
@ 2018-02-01 15:09 ` Geert Uytterhoeven
2018-02-01 15:16 ` Robin Murphy
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2018-02-01 15:09 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] soc: mediatek: Handle return of of_match_device function
2018-02-01 15:09 ` Geert Uytterhoeven
@ 2018-02-01 15:16 ` Robin Murphy
0 siblings, 0 replies; 6+ messages in thread
From: Robin Murphy @ 2018-02-01 15:16 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread