[PATCH net] bonding: initialize err for empty target lists

[PATCH net] bonding: initialize err for empty target lists

[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

Re: [PATCH net] bonding: initialize err for empty target lists

[Nikolay Aleksandrov]

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>

Re: [PATCH net] bonding: initialize err for empty target lists

[Andrew Lunn]

> 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

Re: [PATCH net] bonding: initialize err for empty target lists

[Nikolay Aleksandrov]

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

Re: [PATCH net] bonding: initialize err for empty target lists

[Jay Vosburgh]

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
>

Re: [PATCH net] bonding: initialize err for empty target lists

[Andrew Lunn]

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

Re: [PATCH net] bonding: initialize err for empty target lists

[Hangbin Liu]

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>

Re: [PATCH net] bonding: initialize err for empty target lists

[Jakub Kicinski]

On Fri, 14 Aug 2026 02:44:11 +0200 Andrew Lunn wrote:
> > 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.

FWIW it's a common mistake to run get maint on a file path.
I sent a patch once to print a WARNING when paths are used.
But apparently the path case has at least one important user - Linus :)