All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] bonding: initialize err for empty target lists
@ 2026-08-13 15:31 Ruoyu Wang
  2026-08-13 16:03 ` Nikolay Aleksandrov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ruoyu Wang @ 2026-08-13 15:31 UTC (permalink / raw)
  To: netdev
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, linux-kernel,
	Ruoyu Wang

Empty NLA_NESTED attributes are valid, and bonding uses them to clear
the ARP and NS target lists. When either target attribute is empty,
nla_for_each_nested() does not execute, so err retains an uninitialized
value before it is tested. The request can consequently return an
unpredictable error after clearing the targets.

Initialize err to zero so an empty target list completes successfully.
Non-empty lists still propagate errors from __bond_opt_set() unchanged.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 4fb0ef585eb2 ("bonding: convert arp_ip_target to use the new option API")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/net/bonding/bond_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 4a11572f663d31..87d92d3cce4a88 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -220,7 +220,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct bond_opt_value newval;
 	int miimon = 0;
-	int err;
+	int err = 0;
 
 	if (!data)
 		return 0;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 15:31 [PATCH net] bonding: initialize err for empty target lists Ruoyu Wang
@ 2026-08-13 16:03 ` Nikolay Aleksandrov
  2026-08-13 20:25   ` Andrew Lunn
  2026-08-14  0:31 ` Jay Vosburgh
  2026-08-14  1:23 ` Hangbin Liu
  2 siblings, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2026-08-13 16:03 UTC (permalink / raw)
  To: Ruoyu Wang, netdev
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, linux-kernel

On 13/08/2026 18:31, Ruoyu Wang wrote:
> Empty NLA_NESTED attributes are valid, and bonding uses them to clear
> the ARP and NS target lists. When either target attribute is empty,
> nla_for_each_nested() does not execute, so err retains an uninitialized
> value before it is tested. The request can consequently return an
> unpredictable error after clearing the targets.
> 
> Initialize err to zero so an empty target list completes successfully.
> Non-empty lists still propagate errors from __bond_opt_set() unchanged.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 4fb0ef585eb2 ("bonding: convert arp_ip_target to use the new option API")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>   drivers/net/bonding/bond_netlink.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 4a11572f663d31..87d92d3cce4a88 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -220,7 +220,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>   	struct bonding *bond = netdev_priv(bond_dev);
>   	struct bond_opt_value newval;
>   	int miimon = 0;
> -	int err;
> +	int err = 0;
>   
>   	if (!data)
>   		return 0;

Oh well, I've missed they can be empty. :)
By the way you haven't CCed the Fixes commit author (me).

Thanks,
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 16:03 ` Nikolay Aleksandrov
@ 2026-08-13 20:25   ` Andrew Lunn
  2026-08-13 20:30     ` Nikolay Aleksandrov
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2026-08-13 20:25 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Ruoyu Wang, netdev, jv, andrew+netdev, davem, edumazet, kuba,
	pabeni, linux-kernel

> By the way you haven't CCed the Fixes commit author (me).

Hi Nikolay

./scripts/get_maintainer.pl drivers/net/bonding/bond_netlink.c
Jay Vosburgh <jv@jvosburgh.net> (maintainer:BONDING DRIVER)
Andrew Lunn <andrew+netdev@lunn.ch> (maintainer:NETWORKING DRIVERS)
"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS)
Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
netdev@vger.kernel.org (open list:BONDING DRIVER)
linux-kernel@vger.kernel.org (open list)

You are not listed. Maybe you can submit a patch to MAINTAINERS?

    Andrew

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 20:25   ` Andrew Lunn
@ 2026-08-13 20:30     ` Nikolay Aleksandrov
  2026-08-14  0:44       ` Andrew Lunn
  0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2026-08-13 20:30 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Ruoyu Wang, netdev, jv, andrew+netdev, davem, edumazet, kuba,
	pabeni, linux-kernel

On Thu, Aug 13, 2026 at 10:25:13PM +0200, Andrew Lunn wrote:
> > By the way you haven't CCed the Fixes commit author (me).
> 
> Hi Nikolay
> 
> ./scripts/get_maintainer.pl drivers/net/bonding/bond_netlink.c
> Jay Vosburgh <jv@jvosburgh.net> (maintainer:BONDING DRIVER)
> Andrew Lunn <andrew+netdev@lunn.ch> (maintainer:NETWORKING DRIVERS)
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
> Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS)
> Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> netdev@vger.kernel.org (open list:BONDING DRIVER)
> linux-kernel@vger.kernel.org (open list)
> 
> You are not listed. Maybe you can submit a patch to MAINTAINERS?
> 
>     Andrew

I didn't mean it as a maintainer, it is customary and good practice
to email the author of the commit that introduced the regression,
that is all I was trying to say.

Cheers,
 Nik

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 15:31 [PATCH net] bonding: initialize err for empty target lists Ruoyu Wang
  2026-08-13 16:03 ` Nikolay Aleksandrov
@ 2026-08-14  0:31 ` Jay Vosburgh
  2026-08-14  1:23 ` Hangbin Liu
  2 siblings, 0 replies; 7+ messages in thread
From: Jay Vosburgh @ 2026-08-14  0:31 UTC (permalink / raw)
  To: Ruoyu Wang
  Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
	linux-kernel, Nikolay Aleksandrov

Ruoyu Wang <ruoyuw560@gmail.com> wrote:

>Empty NLA_NESTED attributes are valid, and bonding uses them to clear
>the ARP and NS target lists. When either target attribute is empty,
>nla_for_each_nested() does not execute, so err retains an uninitialized
>value before it is tested. The request can consequently return an
>unpredictable error after clearing the targets.
>
>Initialize err to zero so an empty target list completes successfully.
>Non-empty lists still propagate errors from __bond_opt_set() unchanged.
>
>This issue was found by a static analysis checker and confirmed by manual
>source review.
>
>Fixes: 4fb0ef585eb2 ("bonding: convert arp_ip_target to use the new option API")
>Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>

Acked-by: Jay Vosburgh <jv@jvosburgh.net>


>---
> drivers/net/bonding/bond_netlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 4a11572f663d31..87d92d3cce4a88 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -220,7 +220,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> 	struct bonding *bond = netdev_priv(bond_dev);
> 	struct bond_opt_value newval;
> 	int miimon = 0;
>-	int err;
>+	int err = 0;
> 
> 	if (!data)
> 		return 0;
>-- 
>2.51.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 20:30     ` Nikolay Aleksandrov
@ 2026-08-14  0:44       ` Andrew Lunn
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2026-08-14  0:44 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Ruoyu Wang, netdev, jv, andrew+netdev, davem, edumazet, kuba,
	pabeni, linux-kernel

On Thu, Aug 13, 2026 at 11:30:24PM +0300, Nikolay Aleksandrov wrote:
> On Thu, Aug 13, 2026 at 10:25:13PM +0200, Andrew Lunn wrote:
> > > By the way you haven't CCed the Fixes commit author (me).
> > 
> > Hi Nikolay
> > 
> > ./scripts/get_maintainer.pl drivers/net/bonding/bond_netlink.c
> > Jay Vosburgh <jv@jvosburgh.net> (maintainer:BONDING DRIVER)
> > Andrew Lunn <andrew+netdev@lunn.ch> (maintainer:NETWORKING DRIVERS)
> > "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
> > Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> > Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS)
> > Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> > netdev@vger.kernel.org (open list:BONDING DRIVER)
> > linux-kernel@vger.kernel.org (open list)
> > 
> > You are not listed. Maybe you can submit a patch to MAINTAINERS?
> > 
> >     Andrew
> 
> I didn't mean it as a maintainer, it is customary and good practice
> to email the author of the commit that introduced the regression,
> that is all I was trying to say.

Interesting:

scripts/get_maintainer.pl --help
usage: scripts/get_maintainer.pl [options] patchfile
       scripts/get_maintainer.pl [options] -f file|directory
version: 0.26

MAINTAINER field selection options:
  --email => print email address(es) if any
...
    --fixes => for patches, add signatures of commits with 'Fixes: <commit>' (default: 1 (on))

So it should of done it, if it was run on the patch, rather than the
file.

I wounder what "b4 prep --auto-to-cc" does?

  Andrew

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] bonding: initialize err for empty target lists
  2026-08-13 15:31 [PATCH net] bonding: initialize err for empty target lists Ruoyu Wang
  2026-08-13 16:03 ` Nikolay Aleksandrov
  2026-08-14  0:31 ` Jay Vosburgh
@ 2026-08-14  1:23 ` Hangbin Liu
  2 siblings, 0 replies; 7+ messages in thread
From: Hangbin Liu @ 2026-08-14  1:23 UTC (permalink / raw)
  To: Ruoyu Wang
  Cc: netdev, jv, andrew+netdev, davem, edumazet, kuba, pabeni,
	linux-kernel

On Thu, Aug 13, 2026 at 11:31:26PM +0800, Ruoyu Wang wrote:
> Empty NLA_NESTED attributes are valid, and bonding uses them to clear
> the ARP and NS target lists. When either target attribute is empty,
> nla_for_each_nested() does not execute, so err retains an uninitialized
> value before it is tested. The request can consequently return an
> unpredictable error after clearing the targets.
> 
> Initialize err to zero so an empty target list completes successfully.
> Non-empty lists still propagate errors from __bond_opt_set() unchanged.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 4fb0ef585eb2 ("bonding: convert arp_ip_target to use the new option API")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/net/bonding/bond_netlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 4a11572f663d31..87d92d3cce4a88 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -220,7 +220,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>  	struct bonding *bond = netdev_priv(bond_dev);
>  	struct bond_opt_value newval;
>  	int miimon = 0;
> -	int err;
> +	int err = 0;
>  
>  	if (!data)
>  		return 0;
> -- 
> 2.51.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-14  1:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 15:31 [PATCH net] bonding: initialize err for empty target lists Ruoyu Wang
2026-08-13 16:03 ` Nikolay Aleksandrov
2026-08-13 20:25   ` Andrew Lunn
2026-08-13 20:30     ` Nikolay Aleksandrov
2026-08-14  0:44       ` Andrew Lunn
2026-08-14  0:31 ` Jay Vosburgh
2026-08-14  1:23 ` Hangbin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.