On Thu, Jun 30, 2016 at 3:25 PM, Eric Dumazet <[email protected]> wrote: > On Thu, 2016-06-30 at 15:04 +0200, Dmitry Vyukov wrote: >> On Thu, Jun 30, 2016 at 2:56 PM, Eric Dumazet <[email protected]> wrote: >> > From: Eric Dumazet <[email protected]> >> > >> > 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 >> > >> > Fixes: 815117adaf5b ("bonding: use ether_addr_equal_unaligned for bond >> > addr compare") >> > Fixes: bb54e58929f3 ("bonding: Verify RX LACPDU has proper dest mac-addr") >> > Signed-off-by: Eric Dumazet <[email protected]> >> > Reported-by: Dmitry Vyukov <[email protected]> >> > --- >> > drivers/net/bonding/bond_3ad.c | 11 +++++++---- >> > drivers/net/bonding/bond_alb.c | 3 --- >> > include/net/bonding.h | 7 ++++++- >> > 3 files changed, 13 insertions(+), 8 deletions(-) >> > >> > diff --git a/drivers/net/bonding/bond_3ad.c >> > b/drivers/net/bonding/bond_3ad.c >> > index ca81f46ea1aa..8ad491ab1d01 100644 >> > --- a/drivers/net/bonding/bond_3ad.c >> > +++ b/drivers/net/bonding/bond_3ad.c >> > @@ -101,11 +101,14 @@ enum ad_link_speed_type { >> > #define MAC_ADDRESS_EQUAL(A, B) \ >> > ether_addr_equal_64bits((const u8 *)A, (const u8 *)B) >> > >> > -static struct mac_addr null_mac_addr = { { 0, 0, 0, 0, 0, 0 } }; >> > +static const u8 null_mac_addr[ETH_ALEN] __long_aligned = { >> > + 0, 0, 0, 0, 0, 0 >> > +}; >> >> __long_aligned won't prevent KASAN from barking, it's still 6 bytes >> but we are reading 8. >> Can we please do "ETH_ALEN + 2" as below? > > Really ? > > sizeof(null_mac_addr) is 8, how KASan would decide it is 6 bytes ? > > nm -v drivers/net/bonding/bonding.ko|grep -3 null_mac_addr > > 000000000000084a r __UNIQUE_ID_mode17 > 0000000000000850 r tmpl.46263 > 0000000000000860 r lacpdu_mcast_addr > 0000000000000868 r null_mac_addr > 0000000000000870 r mac_v6_allmcast > 0000000000000878 r mac_bcast > 0000000000000880 r slave_sysfs_ops
What does nm -S say? I've tried to compile a sample program with gcc6 and __attribute__((aligned(8))) does in fact force alignment of start of the symbol, but size still stays 6. (and asan still barks on my test program).
