Re: [PATCH net] bonding: limit BOND_MODE_8023AD to Ethernet devices

[Jay Vosburgh <jv@jvosburgh.net>]

Eric Dumazet <edumazet@google.com> wrote:

>BOND_MODE_8023AD makes sense for ARPHRD_ETHER only.

	Agreed.

	There are likely other ARPHRD_* types that don't make sense,
either.

	Assuming from the szybot logs that an interface named
"ip6gretap" means ARPHRD_IP6GRE, I don't see how ARPHRD_IP6GRE could
work for balance-alb or balance-tlb modes (which expect an Ethernet
header in the packets), and maybe not at all for bonding.

	Still, whether it works usefully is a separate question from
whether it panics, so this patch is certainly correct.

Acked-by: Jay Vosburgh <jv@jvosburgh.net>

	-J

>syzbot reported:
>
> BUG: KASAN: global-out-of-bounds in __hw_addr_create net/core/dev_addr_lists.c:63 [inline]
> BUG: KASAN: global-out-of-bounds in __hw_addr_add_ex+0x25d/0x760 net/core/dev_addr_lists.c:118
>Read of size 16 at addr ffffffff8bf94040 by task syz.1.3580/19497
>
>CPU: 1 UID: 0 PID: 19497 Comm: syz.1.3580 Tainted: G             L      syzkaller #0 PREEMPT(full)
>Tainted: [L]=SOFTLOCKUP
>Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/25/2025
>Call Trace:
> <TASK>
>  dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
>  print_address_description mm/kasan/report.c:378 [inline]
>  print_report+0xca/0x240 mm/kasan/report.c:482
>  kasan_report+0x118/0x150 mm/kasan/report.c:595
> check_region_inline mm/kasan/generic.c:-1 [inline]
>  kasan_check_range+0x2b0/0x2c0 mm/kasan/generic.c:200
>  __asan_memcpy+0x29/0x70 mm/kasan/shadow.c:105
>  __hw_addr_create net/core/dev_addr_lists.c:63 [inline]
>  __hw_addr_add_ex+0x25d/0x760 net/core/dev_addr_lists.c:118
>  __dev_mc_add net/core/dev_addr_lists.c:868 [inline]
>  dev_mc_add+0xa1/0x120 net/core/dev_addr_lists.c:886
>  bond_enslave+0x2b8b/0x3ac0 drivers/net/bonding/bond_main.c:2180
>  do_set_master+0x533/0x6d0 net/core/rtnetlink.c:2963
>  do_setlink+0xcf0/0x41c0 net/core/rtnetlink.c:3165
>  rtnl_changelink net/core/rtnetlink.c:3776 [inline]
>  __rtnl_newlink net/core/rtnetlink.c:3935 [inline]
>  rtnl_newlink+0x161c/0x1c90 net/core/rtnetlink.c:4072
>  rtnetlink_rcv_msg+0x7cf/0xb70 net/core/rtnetlink.c:6958
>  netlink_rcv_skb+0x208/0x470 net/netlink/af_netlink.c:2550
>  netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
>  netlink_unicast+0x82f/0x9e0 net/netlink/af_netlink.c:1344
>  netlink_sendmsg+0x805/0xb30 net/netlink/af_netlink.c:1894
>  sock_sendmsg_nosec net/socket.c:727 [inline]
>  __sock_sendmsg+0x21c/0x270 net/socket.c:742
>  ____sys_sendmsg+0x505/0x820 net/socket.c:2592
>  ___sys_sendmsg+0x21f/0x2a0 net/socket.c:2646
>  __sys_sendmsg+0x164/0x220 net/socket.c:2678
>  do_syscall_32_irqs_on arch/x86/entry/syscall_32.c:83 [inline]
>  __do_fast_syscall_32+0x1dc/0x560 arch/x86/entry/syscall_32.c:307
>  do_fast_syscall_32+0x34/0x80 arch/x86/entry/syscall_32.c:332
> entry_SYSENTER_compat_after_hwframe+0x84/0x8e
> </TASK>
>
>The buggy address belongs to the variable:
> lacpdu_mcast_addr+0x0/0x40
>
>Fixes: 872254dd6b1f ("net/bonding: Enable bonding to enslave non ARPHRD_ETHER")
>Reported-by: syzbot+9c081b17773615f24672@syzkaller.appspotmail.com
>Closes: https://lore.kernel.org/netdev/6966946b.a70a0220.245e30.0002.GAE@google.com/T/#u
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Jay Vosburgh <jv@jvosburgh.net>
>Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>---
> drivers/net/bonding/bond_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 3d56339a8a10d97664e6d8cb8b41a681e6e9efc5..0aca6c937297def91d5740dfd456800432b5e343 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1862,6 +1862,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> 	 */
> 	if (!bond_has_slaves(bond)) {
> 		if (bond_dev->type != slave_dev->type) {
>+			if (slave_dev->type != ARPHRD_ETHER &&
>+			    BOND_MODE(bond) == BOND_MODE_8023AD) {
>+				SLAVE_NL_ERR(bond_dev, slave_dev, extack,
>+					     "8023AD mode requires Ethernet devices");
>+				return -EINVAL;
>+			}
> 			slave_dbg(bond_dev, slave_dev, "change device type from %d to %d\n",
> 				  bond_dev->type, slave_dev->type);
> 
>-- 
>2.52.0.457.g6b5491de43-goog
>

---
	-Jay Vosburgh, jv@jvosburgh.net