Re: [PATCH net-next v5 6/7] bonding: Update for extended arp_ip_target format.

________________________________________
From: Simon Horman <horms@kernel.org>
Sent: Tuesday, July 15, 2025 6:58 AM
To: David Wilder
Cc: netdev@vger.kernel.org; jv@jvosburgh.net; pradeeps@linux.vnet.ibm.com; Pradeep Satyanarayana; i.maximets@ovn.org; Adrian Moreno Zapata; Hangbin Liu
Subject: [EXTERNAL] Re: [PATCH net-next v5 6/7] bonding: Update for extended arp_ip_target format.

quoted

On Mon, Jul 14, 2025 at 03:54:51PM -0700, David Wilder wrote:

quoted

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 <redacted>
---
 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

quoted

quoted

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

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

quoted

Flagged by Smatch.

Hi Simon

Thanks for catching this,  I will make the following change in the next version.

@@ -703,15 +703,14 @@ static int bond_fill_info(struct sk_buff *skb,
        for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
                struct bond_arp_target *target = &bond->params.arp_targets[i];
                struct Data {
-                       __u32 addr;
+                       __be32 addr;
                        struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
                } data;
                int size = 0;

-               if (target->target_ip) {
-                       data.addr = target->target_ip;
-                       size = sizeof(target->target_ip);
-               }
+               BUG_ON(!target->target_ip);