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

Ruoyu Wang posted 1 patch 1 month, 2 weeks ago
drivers/net/bonding/bond_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH net] bonding: initialize err for empty target lists
Posted by Ruoyu Wang 1 month, 2 weeks ago
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
Posted by Hangbin Liu 1 month, 2 weeks ago
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
Posted by Jay Vosburgh 1 month, 2 weeks ago
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
Posted by Nikolay Aleksandrov 1 month, 2 weeks ago
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
Posted by Andrew Lunn 1 month, 2 weeks ago
> 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
Posted by Nikolay Aleksandrov 1 month, 2 weeks ago
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
Posted by Andrew Lunn 1 month, 2 weeks ago
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
Posted by Jakub Kicinski 1 month, 2 weeks ago
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 :)