Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.
In case of NULL print error message and return -ENODEV
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index adaaafc..6a77dea 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2408,6 +2408,11 @@ static int mtk_probe(struct platform_device *pdev)
int i;
match = of_match_device(of_mtk_match, &pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "failed to match device\n");
+ return -ENODEV;
+ }
+
soc = (struct mtk_soc_data *)match->data;
eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
--
2.5.0
On 07/07/2017 09:11 AM, Gustavo A. R. Silva wrote:
> Check return value from call to of_match_device()
> in order to prevent a NULL pointer dereference.
>
> In case of NULL print error message and return -ENODEV
>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
Reviewed-by: Matthias Brugger <[email protected]>
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index adaaafc..6a77dea 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2408,6 +2408,11 @@ static int mtk_probe(struct platform_device *pdev)
> int i;
>
> match = of_match_device(of_mtk_match, &pdev->dev);
> + if (!match) {
> + dev_err(&pdev->dev, "failed to match device\n");
> + return -ENODEV;
> + }
> +
> soc = (struct mtk_soc_data *)match->data;
>
> eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
>
On Fri, Jul 07, 2017 at 02:11:35AM -0500, Gustavo A. R. Silva wrote:
> Check return value from call to of_match_device()
> in order to prevent a NULL pointer dereference.
>
> In case of NULL print error message and return -ENODEV
>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index adaaafc..6a77dea 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2408,6 +2408,11 @@ static int mtk_probe(struct platform_device *pdev)
> int i;
>
> match = of_match_device(of_mtk_match, &pdev->dev);
> + if (!match) {
> + dev_err(&pdev->dev, "failed to match device\n");
> + return -ENODEV;
> + }
> +
> soc = (struct mtk_soc_data *)match->data;
Hi Gustavo
I think you are fixing the wrong thing. The soc variable is unused. Also,
const struct of_device_id of_mtk_match[] = {
{ .compatible = "mediatek,mt2701-eth" },
{},
};
So match->data is NULL. This code is all pointless and should be
removed, rather than try to avoid a NULL pointer dereference which can
not actually happen.
Andrew
Hi Andrew,
Quoting Andrew Lunn <[email protected]>:
> On Fri, Jul 07, 2017 at 02:11:35AM -0500, Gustavo A. R. Silva wrote:
>> Check return value from call to of_match_device()
>> in order to prevent a NULL pointer dereference.
>>
>> In case of NULL print error message and return -ENODEV
>>
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>> ---
>> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> index adaaafc..6a77dea 100644
>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -2408,6 +2408,11 @@ static int mtk_probe(struct platform_device *pdev)
>> int i;
>>
>> match = of_match_device(of_mtk_match, &pdev->dev);
>> + if (!match) {
>> + dev_err(&pdev->dev, "failed to match device\n");
>> + return -ENODEV;
>> + }
>> +
>> soc = (struct mtk_soc_data *)match->data;
>
> Hi Gustavo
>
> I think you are fixing the wrong thing. The soc variable is unused. Also,
>
> const struct of_device_id of_mtk_match[] = {
> { .compatible = "mediatek,mt2701-eth" },
> {},
> };
>
> So match->data is NULL. This code is all pointless and should be
> removed, rather than try to avoid a NULL pointer dereference which can
> not actually happen.
>
You are right. Thank you for pointing it out. I'll remove that code
and send a new patch shortly.
Thanks!
--
Gustavo A. R. Silva
Remove useless local variables _match_, _soc_ and the code related.
Notice that
const struct of_device_id of_mtk_match[] = {
{ .compatible = "mediatek,mt2701-eth" },
{},
};
So match->data is NULL.
Suggested-by: Andrew Lunn <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index adaaafc..b9a5a65 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2401,15 +2401,10 @@ static int mtk_probe(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct device_node *mac_np;
- const struct of_device_id *match;
- struct mtk_soc_data *soc;
struct mtk_eth *eth;
int err;
int i;
- match = of_match_device(of_mtk_match, &pdev->dev);
- soc = (struct mtk_soc_data *)match->data;
-
eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
if (!eth)
return -ENOMEM;
--
2.5.0
Hi, Gustavo
It indeed is useless at the current time point.
but actually I will add new SoC support to the driver in the next week,
which requires the variable match :-(
Sean
On Fri, 2017-07-07 at 15:23 -0500, Gustavo A. R. Silva wrote:
> Remove useless local variables _match_, _soc_ and the code related.
>
> Notice that
>
> const struct of_device_id of_mtk_match[] = {
> { .compatible = "mediatek,mt2701-eth" },
> {},
> };
>
> So match->data is NULL.
>
> Suggested-by: Andrew Lunn <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index adaaafc..b9a5a65 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2401,15 +2401,10 @@ static int mtk_probe(struct platform_device *pdev)
> {
> struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> struct device_node *mac_np;
> - const struct of_device_id *match;
> - struct mtk_soc_data *soc;
> struct mtk_eth *eth;
> int err;
> int i;
>
> - match = of_match_device(of_mtk_match, &pdev->dev);
> - soc = (struct mtk_soc_data *)match->data;
> -
> eth = devm_kzalloc(&pdev->dev, sizeof(*eth), GFP_KERNEL);
> if (!eth)
> return -ENOMEM;
From: "Gustavo A. R. Silva" <[email protected]>
Date: Fri, 7 Jul 2017 15:23:34 -0500
> Remove useless local variables _match_, _soc_ and the code related.
>
> Notice that
>
> const struct of_device_id of_mtk_match[] = {
> { .compatible = "mediatek,mt2701-eth" },
> {},
> };
>
> So match->data is NULL.
>
> Suggested-by: Andrew Lunn <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
Applied, thanks.
If someone needs this they can it back, in a less buggy form.