Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bonding: prevent out of bound accesses

ether_addr_equal_64bits() requires some care about its arguments,
namely that 8 bytes might be read, even if last 2 byte values are not
used.

KASan detected a violation with null_mac_addr and lacpdu_mcast_addr
in bond_3ad.c

Same problem with mac_bcast[] and mac_v6_allmcast[] in bond_alb.c :
Although the 8-byte alignment was there, KASan would detect out
of bound accesses.

Fixes: 815117adaf5b ("bonding: use ether_addr_equal_unaligned for bond addr compare")
Fixes: bb54e58929f3 ("bonding: Verify RX LACPDU has proper dest mac-addr")
Fixes: 885a136c52a8 ("bonding: use compare_ether_addr_64bits() in ALB")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Acked-by: Dmitry Vyukov <dvyukov@google.com>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Ding Tianhong <dingtianhong@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Eric Dumazet and committed by
David S. Miller
f87fda00 3d8c4530

+15 -10
+7 -4
drivers/net/bonding/bond_3ad.c
··· 101 101 #define MAC_ADDRESS_EQUAL(A, B) \ 102 102 ether_addr_equal_64bits((const u8 *)A, (const u8 *)B) 103 103 104 - static struct mac_addr null_mac_addr = { { 0, 0, 0, 0, 0, 0 } }; 104 + static const u8 null_mac_addr[ETH_ALEN + 2] __long_aligned = { 105 + 0, 0, 0, 0, 0, 0 106 + }; 105 107 static u16 ad_ticks_per_sec; 106 108 static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000; 107 109 108 - static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR; 110 + static const u8 lacpdu_mcast_addr[ETH_ALEN + 2] __long_aligned = 111 + MULTICAST_LACPDU_ADDR; 109 112 110 113 /* ================= main 802.3ad protocol functions ================== */ 111 114 static int ad_lacpdu_send(struct port *port); ··· 1742 1739 aggregator->is_individual = false; 1743 1740 aggregator->actor_admin_aggregator_key = 0; 1744 1741 aggregator->actor_oper_aggregator_key = 0; 1745 - aggregator->partner_system = null_mac_addr; 1742 + eth_zero_addr(aggregator->partner_system.mac_addr_value); 1746 1743 aggregator->partner_system_priority = 0; 1747 1744 aggregator->partner_oper_aggregator_key = 0; 1748 1745 aggregator->receive_state = 0; ··· 1764 1761 if (aggregator) { 1765 1762 ad_clear_agg(aggregator); 1766 1763 1767 - aggregator->aggregator_mac_address = null_mac_addr; 1764 + eth_zero_addr(aggregator->aggregator_mac_address.mac_addr_value); 1768 1765 aggregator->aggregator_identifier = 0; 1769 1766 aggregator->slave = NULL; 1770 1767 }
+2 -5
drivers/net/bonding/bond_alb.c
··· 42 42 43 43 44 44 45 - #ifndef __long_aligned 46 - #define __long_aligned __attribute__((aligned((sizeof(long))))) 47 - #endif 48 - static const u8 mac_bcast[ETH_ALEN] __long_aligned = { 45 + static const u8 mac_bcast[ETH_ALEN + 2] __long_aligned = { 49 46 0xff, 0xff, 0xff, 0xff, 0xff, 0xff 50 47 }; 51 - static const u8 mac_v6_allmcast[ETH_ALEN] __long_aligned = { 48 + static const u8 mac_v6_allmcast[ETH_ALEN + 2] __long_aligned = { 52 49 0x33, 0x33, 0x00, 0x00, 0x00, 0x01 53 50 }; 54 51 static const int alb_delta_in_ticks = HZ / ALB_TIMER_TICKS_PER_SEC;
+6 -1
include/net/bonding.h
··· 34 34 35 35 #define BOND_DEFAULT_MIIMON 100 36 36 37 + #ifndef __long_aligned 38 + #define __long_aligned __attribute__((aligned((sizeof(long))))) 39 + #endif 37 40 /* 38 41 * Less bad way to call ioctl from within the kernel; this needs to be 39 42 * done some other way to get the call out of interrupt context. ··· 141 138 struct reciprocal_value reciprocal_packets_per_slave; 142 139 u16 ad_actor_sys_prio; 143 140 u16 ad_user_port_key; 144 - u8 ad_actor_system[ETH_ALEN]; 141 + 142 + /* 2 bytes of padding : see ether_addr_equal_64bits() */ 143 + u8 ad_actor_system[ETH_ALEN + 2]; 145 144 }; 146 145 147 146 struct bond_parm_tbl {