netdev - Re: [PATCH net-next v5 6/7] bonding: Update for extended arp_ip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250715135821.GY721198@horms.kernel.org>
Date: Tue, 15 Jul 2025 14:58:21 +0100
From: Simon Horman <horms@...nel.org>
To: David Wilder <wilder@...ibm.com>
Cc: netdev@...r.kernel.org, jv@...sburgh.net, pradeeps@...ux.vnet.ibm.com,
	pradeep@...ibm.com, i.maximets@....org, amorenoz@...hat.com,
	haliu@...hat.com
Subject: Re: [PATCH net-next v5 6/7] bonding: Update for extended
 arp_ip_target format.

On Mon, Jul 14, 2025 at 03:54:51PM -0700, David Wilder wrote:
> Updated bond_fill_info() to support extended arp_ip_target format.
> 
> Forward and backward compatibility between the kernel and iprout2 is
> preserved.
> 
> Signed-off-by: David Wilder <wilder@...ibm.com>
> ---
>  drivers/net/bonding/bond_netlink.c | 28 ++++++++++++++++++++++++++--
>  include/net/bonding.h              |  1 +
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 5486ef40907e..6e8aebe5629f 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -701,8 +701,32 @@ static int bond_fill_info(struct sk_buff *skb,
>  
>  	targets_added = 0;
>  	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
> -		if (bond->params.arp_targets[i].target_ip) {
> -			if (nla_put_be32(skb, i, bond->params.arp_targets[i].target_ip))
> +		struct bond_arp_target *target = &bond->params.arp_targets[i];
> +		struct Data {
> +			__u32 addr;
> +			struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
> +		} data;
> +		int size = 0;
> +
> +		if (target->target_ip) {
> +			data.addr = target->target_ip;

Hi David,

There appears to be an endian mismatch here. Sparse says:

  .../bond_netlink.c:712:35: warning: incorrect type in assignment (different base types)
  .../bond_netlink.c:712:35:    expected unsigned int [usertype] addr
  .../bond_netlink.c:712:35:    got restricted __be32 [usertype] target_ip

> +			size = sizeof(target->target_ip);
> +		}

It seems that data.addr may be used uninitialised below
if the if condition above is not met.

Flagged by Smatch.

> +
> +		for (int level = 0; target->flags & BOND_TARGET_USERTAGS && target->tags; level++) {
> +			if (level > BOND_MAX_VLAN_TAGS)
> +				goto nla_put_failure;
> +
> +			memcpy(&data.vlans[level], &target->tags[level],
> +			       sizeof(struct bond_vlan_tag));
> +			size = size + sizeof(struct bond_vlan_tag);
> +
> +			if (target->tags[level].vlan_proto == BOND_VLAN_PROTO_NONE)
> +				break;
> +		}
> +
> +		if (size) {
> +			if (nla_put(skb, i, size, &data))
>  				goto nla_put_failure;
>  			targets_added = 1;
>  		}

...

-- 
pw-bot: changes-requested