netdev - RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Oct 2017 00:21:56 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     "vyasevic@...hat.com" <vyasevic@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "Malek, Patryk" <patryk.malek@...el.com>
Subject: RE: removing bridge in vlan_filtering mode requests delete of
 attached ports main MAC address

> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, October 26, 2017 1:33 PM
> To: Keller, Jacob E <jacob.e.keller@...el.com>; vyasevic@...hat.com;
> netdev@...r.kernel.org
> Cc: Malek, Patryk <patryk.malek@...el.com>
> Subject: RE: removing bridge in vlan_filtering mode requests delete of attached
> ports main MAC address
> 
> > -----Original Message-----
> > From: netdev-owner@...r.kernel.org [mailto:netdev-
> owner@...r.kernel.org]
> > On Behalf Of Keller, Jacob E
> > Sent: Thursday, October 26, 2017 1:27 PM
> > To: vyasevic@...hat.com; netdev@...r.kernel.org
> > Cc: Malek, Patryk <patryk.malek@...el.com>
> > Subject: RE: removing bridge in vlan_filtering mode requests delete of attached
> > ports main MAC address
> >
> > > -----Original Message-----
> > > From: Vlad Yasevich [mailto:vyasevic@...hat.com]
> > > Sent: Thursday, October 26, 2017 3:22 AM
> > > To: Keller, Jacob E <jacob.e.keller@...el.com>; netdev@...r.kernel.org
> > > Cc: Malek, Patryk <patryk.malek@...el.com>
> > > Subject: Re: removing bridge in vlan_filtering mode requests delete of
> attached
> > > ports main MAC address
> > >
> > > Hi Jake
> > >
> > > I think adding a !fdb->local should work.  local fdb contain the address of
> > assigned
> > > to
> > > the ports of the bridge and those shouldn't be directly removed.
> > >
> > > If that works,  that looks like the right solution.
> > >
> > > -vlad
> > >
> >
> > So this does prevent us from removing the port's address. However, if I add
> two
> > devices to the bridge, then after removing the bridge, each device now keeps
> > both permanent addresses in their list, which isn't what we want is it?
> >
> > Do we even want to assign the local fdb addresses to every port?
> >
> > Obviously, I don't fully understand this code, so I think I'm missing something
> > here.
> >
> > Regards,
> > Jake
> >
> 
> Ok, I tried this again, and it didn't end up crossing the local device addresses to
> each port. I'm not sure how that happened the first time yet, so maybe it is
> correct to skip removing local addresses... but if we skip removing them, wouldn't
> we want to skip adding them too?
> 
> Thanks,
> Jake

I'm still digging into this. It turns out adding two devices, enabling vlan filtering, and deleting the bridge sometimes (but not always, not sure what condition triggers it) causes the hw address of one of the devices to be assigned to the other device.

I'm still unsure whether sync_static should be assigning local addresses to each device, but it appears like it should. In this case, I'm really unsure how to handle this case properly.

If we add local addresses, we need to delete the ones that aren't specific to that device so that after removing the bridge we end up in the original configuration.. but I'm not really sure how best to do this.

Using !fdb->is_local in unsync_static works to resolve my issue, but I believe it papers over other issues, since it means that we'll never delete static addresses when deleting the ports or exiting promiscuous mode.

I think checking fdb->dst might work, but that would break if we manually add a new address and tag is as permanent, see line 806 of br_fdb.c... In this case, we'd never delete this address even though it was not originally on the device.

I checked other drivers, and it turns out that at least one (ixgbe) doesn't have this problem because the hw address is special and isn't actually stored in a hardware MAC filter list. In i40e we keep the hardware address in the same list as all the other MAC filters.

We could "fix" this in i40e by treating the hw permanent address separately and essentially ignoring it from the dev_uc_del() calls.. but I still feel like this papers over the issues in the bridge code.

Any thoughts or suggestions? I haven't checked other drivers to see how they handle addresses in the unicast table (whether they treat the hw address as special or not, like ixgbe ultimately does).

Thanks,
Jake