[PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[Kangjie Lu]

of_get_phy_mode may fail and return a negative error code;
the fix checks the return value of of_get_phy_mode and
returns NULL of it fails.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 drivers/net/ethernet/renesas/sh_eth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 339b2eae2100..239eeafe1b2d 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3187,6 +3187,8 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
 		return NULL;
 
 	pdata->phy_interface = of_get_phy_mode(np);
+	if (pdata->phy_interface < 0)
+		return NULL;
 
 	mac_addr = of_get_mac_address(np);
 	if (mac_addr)
-- 
2.17.1

Re: [PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[Sergei Shtylyov]

Hello!

On 03/11/2019 10:49 AM, Kangjie Lu wrote:

> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.

  Sorry, I overlooked this issue.

> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Fixes: b356e978e92f ("sh_eth: add device tree support")
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> ---
>  drivers/net/ethernet/renesas/sh_eth.c | 2 ++
>  1 file changed, 2 insertions(+)

   Why are you not fixing ravb_main.c as well? Especially as you have "renesas:"
in your subject, not "sh_eth:"? :-/

[...]

MBR, Sergei

Re: [PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Mon, 11 Mar 2019 13:29:10 +0300

> Hello!
> 
> On 03/11/2019 10:49 AM, Kangjie Lu wrote:
> 
>> of_get_phy_mode may fail and return a negative error code;
>> the fix checks the return value of of_get_phy_mode and
>> returns NULL of it fails.
> 
>   Sorry, I overlooked this issue.
> 
>> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> 
> Fixes: b356e978e92f ("sh_eth: add device tree support")
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
>> ---
>>  drivers/net/ethernet/renesas/sh_eth.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
>    Why are you not fixing ravb_main.c as well? Especially as you have "renesas:"
> in your subject, not "sh_eth:"? :-/

Kangjie please fix the subject line and incorporate Sergei's Fixes and
Reviewed-by tags.

Thanks.

Re: [PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[Sergei Shtylyov]

On 03/11/2019 08:52 PM, David Miller wrote:

>>> of_get_phy_mode may fail and return a negative error code;
>>> the fix checks the return value of of_get_phy_mode and
>>> returns NULL of it fails.
>>
>>   Sorry, I overlooked this issue.
>>
>>> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
>>
>> Fixes: b356e978e92f ("sh_eth: add device tree support")
>> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>>> ---
>>>  drivers/net/ethernet/renesas/sh_eth.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>
>>    Why are you not fixing ravb_main.c as well? Especially as you have "renesas:"
>> in your subject, not "sh_eth:"? :-/
> 
> Kangjie please fix the subject line and incorporate Sergei's Fixes and
> Reviewed-by tags.

   For the record, this patch was a nop, I have to NAK it. Kangjie, please address
Geert's feedback, thanks.

> Thanks.

MBR, Sergei

[PATCH v2] net: sh_eth: fix a missing check of of_get_phy_mode

[Kangjie Lu]

of_get_phy_mode may fail and return a negative error code;
the fix checks the return value of of_get_phy_mode and
returns NULL of it fails.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 drivers/net/ethernet/renesas/sh_eth.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 239eeafe1b2d..e33af371b169 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3181,14 +3181,16 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
 	struct device_node *np = dev->of_node;
 	struct sh_eth_plat_data *pdata;
 	const char *mac_addr;
+	int ret;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return NULL;
 
-	pdata->phy_interface = of_get_phy_mode(np);
-	if (pdata->phy_interface < 0)
+	ret = of_get_phy_mode(np);
+	if (ret < 0)
 		return NULL;
+	pdata->phy_interface = ret;
 
 	mac_addr = of_get_mac_address(np);
 	if (mac_addr)
-- 
2.17.1

Re: [PATCH v2] net: sh_eth: fix a missing check of of_get_phy_mode

[Sergei Shtylyov]

Hello!

On 12.03.2019 9:30, Kangjie Lu wrote:

> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 239eeafe1b2d..e33af371b169 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -3181,14 +3181,16 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
>   	struct device_node *np = dev->of_node;
>   	struct sh_eth_plat_data *pdata;
>   	const char *mac_addr;
> +	int ret;
>   
>   	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>   	if (!pdata)
>   		return NULL;
>   
> -	pdata->phy_interface = of_get_phy_mode(np);
> -	if (pdata->phy_interface < 0)

    Please generate your patch against the pristine net.git, not atop
of your previous version.

> +	ret = of_get_phy_mode(np);
> +	if (ret < 0)
>   		return NULL;
> +	pdata->phy_interface = ret;
>   
>   	mac_addr = of_get_mac_address(np);
>   	if (mac_addr)
> 

MBR, Sergei

[PATCH v3] net: sh_eth: fix a missing check of of_get_phy_mode

[Kangjie Lu]

of_get_phy_mode may fail and return a negative error code;
the fix checks the return value of of_get_phy_mode and
returns NULL of it fails.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 drivers/net/ethernet/renesas/sh_eth.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 339b2eae2100..e33af371b169 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3181,12 +3181,16 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
 	struct device_node *np = dev->of_node;
 	struct sh_eth_plat_data *pdata;
 	const char *mac_addr;
+	int ret;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return NULL;
 
-	pdata->phy_interface = of_get_phy_mode(np);
+	ret = of_get_phy_mode(np);
+	if (ret < 0)
+		return NULL;
+	pdata->phy_interface = ret;
 
 	mac_addr = of_get_mac_address(np);
 	if (mac_addr)
-- 
2.17.1

Re: [PATCH v3] net: sh_eth: fix a missing check of of_get_phy_mode

[Sergei Shtylyov]

On 12.03.2019 10:43, Kangjie Lu wrote:

> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Fixes: b356e978e92f ("sh_eth: add device tree support")
Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> ---

    It's a good practice to describe the changes between the patch versions here.

[...]

MBR, Sergei

Re: [PATCH v3] net: sh_eth: fix a missing check of of_get_phy_mode

[Geert Uytterhoeven]

Hi Kangjie,

On Tue, Mar 12, 2019 at 8:43 AM Kangjie Lu <kjlu@umn.edu> wrote:
> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Thanks for the update!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v3] net: sh_eth: fix a missing check of of_get_phy_mode

[David Miller]

From: Kangjie Lu <kjlu@umn.edu>
Date: Tue, 12 Mar 2019 02:43:18 -0500

> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Applied with Fixes: tag added.

Re: [PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[Geert Uytterhoeven]

Hi Kangjie,

On Mon, Mar 11, 2019 at 8:50 AM Kangjie Lu <kjlu@umn.edu> wrote:
> of_get_phy_mode may fail and return a negative error code;
> the fix checks the return value of of_get_phy_mode and
> returns NULL of it fails.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -3187,6 +3187,8 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
>                 return NULL;
>
>         pdata->phy_interface = of_get_phy_mode(np);
> +       if (pdata->phy_interface < 0)
> +               return NULL;

sh_eth_plat_data.phy_interface has type phy_interface_t.
This is an enum containing only positive values, hence it is unsigned.
So the condition can never be true.

of_get_phy_mode() returns int, as it has to indicate errors by returning
negative error codes.  Hence please use an intermediate of type int:

    int ret;

    ...
    ret = of_get_phy_mode(np);
    if (ret < 0)
            return NULL;

    pdata->phy_interface = ret;

>
>         mac_addr = of_get_mac_address(np);
>         if (mac_addr)

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

Re: [PATCH v2] net: renesas: fix a missing check of of_get_phy_mode

[Sergei Shtylyov]

On 03/11/2019 01:59 PM, Geert Uytterhoeven wrote:

>> of_get_phy_mode may fail and return a negative error code;
>> the fix checks the return value of of_get_phy_mode and
>> returns NULL of it fails.
>>
>> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> 
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -3187,6 +3187,8 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
>>                 return NULL;
>>
>>         pdata->phy_interface = of_get_phy_mode(np);
>> +       if (pdata->phy_interface < 0)
>> +               return NULL;
> 
> sh_eth_plat_data.phy_interface has type phy_interface_t.
> This is an enum containing only positive values, hence it is unsigned.
> So the condition can never be true.

   Good catch, thank you!
   My gcc doesn't warn but doesn't generate the code for the branch either...

[...]

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei