netdev - Re: [RFC PATCH 3/9] net: dsa: convert check for 802.1Q upper when bridged into PRECHANGEUPPER

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4bfcb189-b790-ccf9-d1d7-620624ea528e@gmail.com>
Date:   Sat, 19 Sep 2020 19:34:17 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        davem@...emloft.net
Cc:     andrew@...n.ch, vivien.didelot@...il.com, idosch@...sch.org,
        jiri@...nulli.us, kurt.kanzenbach@...utronix.de, kuba@...nel.org
Subject: Re: [RFC PATCH 3/9] net: dsa: convert check for 802.1Q upper when
 bridged into PRECHANGEUPPER



On 9/19/2020 6:47 PM, Vladimir Oltean wrote:
> DSA tries to prevent having a VLAN added by a bridge and by an 802.1Q
> upper at the same time. It does that by checking the VID in
> .ndo_vlan_rx_add_vid(), since that's something that the 8021q module
> calls, via vlan_vid_add(). When a VLAN matches in both subsystems, this
> check returns -EBUSY.
> 
> However the vlan_vid_add() function isn't specific to the 8021q module
> in any way at all. It is simply the kernel's way to tell an interface to
> add a VLAN to its RX filter and not drop that VLAN. So there's no reason
> to return -EBUSY when somebody tries to call vlan_vid_add() for a VLAN
> that was installed by the bridge. The proper behavior is to accept that
> configuration.
> 
> So what's wrong is how DSA checks that it has an 8021q upper. It should
> look at the actual uppers for that, not just assume that the 8021q
> module was somewhere in the call stack of .ndo_vlan_rx_add_vid().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian