netdev - Re: [PATCH net-next v13 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: <ef443366-f841-4a84-9409-818fc31b2c0c@redhat.com>
Date: Thu, 16 Oct 2025 13:50:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: David Wilder <wilder@...ibm.com>, netdev@...r.kernel.org,
 Jakub Kicinski <kuba@...nel.org>
Cc: jv@...sburgh.net, pradeep@...ibm.com, i.maximets@....org,
 amorenoz@...hat.com, haliu@...hat.com, stephen@...workplumber.org,
 horms@...nel.org, andrew+netdev@...n.ch, edumazet@...gle.com
Subject: Re: [PATCH net-next v13 6/7] bonding: Update for extended
 arp_ip_target format.

On 10/14/25 1:52 AM, David Wilder wrote:
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 15782745fa4d..d1946d387e95 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -678,6 +678,7 @@ static int bond_fill_info(struct sk_buff *skb,
>  			  const struct net_device *bond_dev)
>  {
>  	struct bonding *bond = netdev_priv(bond_dev);
> +	struct bond_arp_target *arptargets;
>  	unsigned int packets_per_slave;
>  	int ifindex, i, targets_added;
>  	struct nlattr *targets;
> @@ -716,12 +717,31 @@ static int bond_fill_info(struct sk_buff *skb,
>  		goto nla_put_failure;
>  
>  	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))
> -				goto nla_put_failure;
> -			targets_added = 1;
> +
> +	arptargets = bond->params.arp_targets;
> +	for (i = 0; i < BOND_MAX_ARP_TARGETS && arptargets[i].target_ip ; i++) {
> +		struct data {
> +			__be32 addr;
> +			struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
> +		} __packed data;
> +		int level, size;
> +
> +		data.addr = arptargets[i].target_ip;
> +		size = sizeof(__be32);
> +		targets_added = 1;
> +
> +		if (arptargets[i].flags & BOND_TARGET_USERTAGS) {
> +			for (level = 0; level < BOND_MAX_VLAN_TAGS + 1 ; level++) {
> +				data.vlans[level].vlan_proto = arptargets[i].tags[level].vlan_proto;
> +				data.vlans[level].vlan_id = arptargets[i].tags[level].vlan_id;
> +				size = size + sizeof(struct bond_vlan_tag);
> +				if (arptargets[i].tags[level].vlan_proto == BOND_VLAN_PROTO_NONE)
> +					break;
> +				}

Apparently there is an extra tab above. This has been already noted in a
previous revision.

>  		}
> +
> +		if (nla_put(skb, i, size, &data))
> +			goto nla_put_failure;
>  	}
>  
>  	if (targets_added)

I guess you should update bond_get_size() accordingly???

Also changing the binary layout of an existing NL type does not feel
safe. @Jakub: is that something we can safely allow?

/P