[PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Liang He]

In prestera_port_sfp_bind(), there are two refcounting bugs:
(1) we should call of_node_get() before of_find_node_by_name() as
it will automaitcally decrease the refcount of 'from' argument;
(2) we should call of_node_put() for the break of the iteration
for_each_child_of_node() as it will automatically increase and
decrease the 'child'.

Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
Signed-off-by: Liang He <windhl@126.com>
---
 it will be safe to call of_node_put() at the end of the function as
the of_node_put() can handle NULL.

 drivers/net/ethernet/marvell/prestera/prestera_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index ede3e53b9790..a895862b4821 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -368,6 +368,7 @@ static int prestera_port_sfp_bind(struct prestera_port *port)
 	if (!sw->np)
 		return 0;
 
+	of_node_get(sw->np);
 	ports = of_find_node_by_name(sw->np, "ports");
 
 	for_each_child_of_node(ports, node) {
@@ -417,6 +418,7 @@ static int prestera_port_sfp_bind(struct prestera_port *port)
 	}
 
 out:
+	of_node_put(node);
 	of_node_put(ports);
 	return err;
 }
-- 
2.25.1

Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Jakub Kicinski]

On Thu, 15 Sep 2022 12:06:55 +0800 Liang He wrote:
> In prestera_port_sfp_bind(), there are two refcounting bugs:
> (1) we should call of_node_get() before of_find_node_by_name() as
> it will automaitcally decrease the refcount of 'from' argument;
> (2) we should call of_node_put() for the break of the iteration
> for_each_child_of_node() as it will automatically increase and
> decrease the 'child'.
> 
> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
> Signed-off-by: Liang He <windhl@126.com>

Please repost and CC all the authors of the patch under Fixes.

Re:Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Liang He]

At 2022-09-21 08:45:29, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Thu, 15 Sep 2022 12:06:55 +0800 Liang He wrote:
>> In prestera_port_sfp_bind(), there are two refcounting bugs:
>> (1) we should call of_node_get() before of_find_node_by_name() as
>> it will automaitcally decrease the refcount of 'from' argument;
>> (2) we should call of_node_put() for the break of the iteration
>> for_each_child_of_node() as it will automatically increase and
>> decrease the 'child'.
>> 
>> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
>> Signed-off-by: Liang He <windhl@126.com>
>
>Please repost and CC all the authors of the patch under Fixes.

Thanks for your reply, Jakub

As I was the only one author, you mean following tag format:

""
Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
CC: Liang He <windhl@126.com>
Signed-off-by: Liang He <windhl@126.com>
""

If there is anything wrong, sorry for this trouble.

Thanks,

Liang

Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Jakub Kicinski]

On Wed, 21 Sep 2022 17:02:52 +0800 (CST) Liang He wrote:
> At 2022-09-21 08:45:29, "Jakub Kicinski" <kuba@kernel.org> wrote:
> >On Thu, 15 Sep 2022 12:06:55 +0800 Liang He wrote:  
> >> In prestera_port_sfp_bind(), there are two refcounting bugs:
> >> (1) we should call of_node_get() before of_find_node_by_name() as
> >> it will automaitcally decrease the refcount of 'from' argument;
> >> (2) we should call of_node_put() for the break of the iteration
> >> for_each_child_of_node() as it will automatically increase and
> >> decrease the 'child'.
> >> 
> >> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
> >> Signed-off-by: Liang He <windhl@126.com>  
> >
> >Please repost and CC all the authors of the patch under Fixes.  
> 
> Thanks for your reply, Jakub
> 
> As I was the only one author, you mean following tag format:
> 
> ""
> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
> CC: Liang He <windhl@126.com>
> Signed-off-by: Liang He <windhl@126.com>

No, no, CC the authors of the patch under fixes, which is to say -
the patch which introduced the problem. The fixes tag you have is:

Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")

so:

commit 52323ef75414d60b17f683076833eb55a6bffa2b
Author: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Date:   Tue Jul 19 13:57:16 2022 +0300

    net: marvell: prestera: add phylink support
    
    For SFP port prestera driver will use kernel
    phylink infrastucture to configure port mode based on
    the module that has beed inserted
    
    Co-developed-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
    Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
    Co-developed-by: Taras Chornyi <taras.chornyi@plvision.eu>
    Signed-off-by: Taras Chornyi <taras.chornyi@plvision.eu>
    Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
    Signed-off-by: David S. Miller <davem@davemloft.net>

I was asking to also CC Yevhen and Oleksandr. Taras seems already CCed.

Re:Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Liang He]

At 2022-09-21 20:36:35, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Wed, 21 Sep 2022 17:02:52 +0800 (CST) Liang He wrote:
>> At 2022-09-21 08:45:29, "Jakub Kicinski" <kuba@kernel.org> wrote:
>> >On Thu, 15 Sep 2022 12:06:55 +0800 Liang He wrote:  
>> >> In prestera_port_sfp_bind(), there are two refcounting bugs:
>> >> (1) we should call of_node_get() before of_find_node_by_name() as
>> >> it will automaitcally decrease the refcount of 'from' argument;
>> >> (2) we should call of_node_put() for the break of the iteration
>> >> for_each_child_of_node() as it will automatically increase and
>> >> decrease the 'child'.
>> >> 
>> >> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
>> >> Signed-off-by: Liang He <windhl@126.com>  
>> >
>> >Please repost and CC all the authors of the patch under Fixes.  
>> 
>> Thanks for your reply, Jakub
>> 
>> As I was the only one author, you mean following tag format:
>> 
>> ""
>> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
>> CC: Liang He <windhl@126.com>
>> Signed-off-by: Liang He <windhl@126.com>
>
>No, no, CC the authors of the patch under fixes, which is to say -
>the patch which introduced the problem. The fixes tag you have is:
>
>Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
>
>so:
>
>commit 52323ef75414d60b17f683076833eb55a6bffa2b
>Author: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
>Date:   Tue Jul 19 13:57:16 2022 +0300
>
>    net: marvell: prestera: add phylink support
>    
>    For SFP port prestera driver will use kernel
>    phylink infrastucture to configure port mode based on
>    the module that has beed inserted
>    
>    Co-developed-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
>    Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
>    Co-developed-by: Taras Chornyi <taras.chornyi@plvision.eu>
>    Signed-off-by: Taras Chornyi <taras.chornyi@plvision.eu>
>    Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
>    Signed-off-by: David S. Miller <davem@davemloft.net>
>

>I was asking to also CC Yevhen and Oleksandr. Taras seems already CCed.


Thanks very much, I will follow this rule in my future patches.

Liang

[PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Liang He]

In prestera_port_sfp_bind(), there are two refcounting bugs:
(1) we should call of_node_get() before of_find_node_by_name() as
it will automaitcally decrease the refcount of 'from' argument;
(2) we should call of_node_put() for the break of the iteration
for_each_child_of_node() as it will automatically increase and
decrease the 'child'.

Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
Signed-off-by: Liang He <windhl@126.com>
---
 it will be safe to call of_node_put() at the end of the function as
the of_node_put() can handle NULL.

 drivers/net/ethernet/marvell/prestera/prestera_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index ede3e53b9790..a895862b4821 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -368,6 +368,7 @@ static int prestera_port_sfp_bind(struct prestera_port *port)
 	if (!sw->np)
 		return 0;
 
+	of_node_get(sw->np);
 	ports = of_find_node_by_name(sw->np, "ports");
 
 	for_each_child_of_node(ports, node) {
@@ -417,6 +418,7 @@ static int prestera_port_sfp_bind(struct prestera_port *port)
 	}
 
 out:
+	of_node_put(node);
 	of_node_put(ports);
 	return err;
 }
-- 
2.25.1

Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[Yevhen Orlov]

On Wed, Sep 21, 2022 at 09:32:45PM +0800, Liang He wrote:
> In prestera_port_sfp_bind(), there are two refcounting bugs:
> (1) we should call of_node_get() before of_find_node_by_name() as
> it will automaitcally decrease the refcount of 'from' argument;
> (2) we should call of_node_put() for the break of the iteration
> for_each_child_of_node() as it will automatically increase and
> decrease the 'child'.
> 
> Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
> Signed-off-by: Liang He <windhl@126.com>
> ---
>  it will be safe to call of_node_put() at the end of the function as
> the of_node_put() can handle NULL.
> 
>  drivers/net/ethernet/marvell/prestera/prestera_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Yevhen Orlov <yevhen.orlov@plvision.eu>

Re: [PATCH] net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 21 Sep 2022 21:32:45 +0800 you wrote:
> In prestera_port_sfp_bind(), there are two refcounting bugs:
> (1) we should call of_node_get() before of_find_node_by_name() as
> it will automaitcally decrease the refcount of 'from' argument;
> (2) we should call of_node_put() for the break of the iteration
> for_each_child_of_node() as it will automatically increase and
> decrease the 'child'.
> 
> [...]

Here is the summary with links:
  - net: marvell: Fix refcounting bugs in prestera_port_sfp_bind()
    https://git.kernel.org/netdev/net/c/3aac7ada64d8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html