linux-kernel - [PATCH RFC] net: dsa: Make switches VLAN aware when enslaved into a bridge

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-next>] [day] [month] [year] [list]

Message-Id: <20181024193657.24012-1-f.fainelli@gmail.com>
Date:   Wed, 24 Oct 2018 12:36:57 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     netdev@...r.kernel.org
Cc:     jiri@...lanox.com, petr@...lanox.com, idosch@...lanox.com,
        privat@...l-hjelmeland.no, Woojung.Huh@...rochip.com,
        tristram.ha@...rochip.com, Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org (open list)
Subject: [PATCH RFC] net: dsa: Make switches VLAN aware when enslaved into a bridge

Commit 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is
disabled") changed the behavior of DSA switches when the switch ports
are enslaved into the bridge and only pushed the VLAN configuration down
to the switch if the bridge is configured with VLAN filtering enabled.

This is unfortunately wrong, because what vlan_filtering configures is a
policy on the acceptance of VLAN tagged frames with an unknown VID.

vlan_filtering=0 means a frame with a VLAN tag that is not part of the
VLAN table should be allowed to ingress the switch, and vlan_fltering=1
would reject that frame.

Fixes: 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
---
Andrew,

I checked with Jiri and he confirmed that our interpretention of
vlan_filtering in DSA was incorrect and that it does denote whether the
switch should be doing VID ingress policy checking.

You mentioned in the commit message some problems without being too
specific about them which is why I am putting the same checks in
mv88e6xxx in order not to break your use cases. Let me know if you want
to drop that hunk entirely.

Thanks!

 drivers/net/dsa/mv88e6xxx/chip.c |  7 ++++++-
 net/dsa/port.c                   | 10 ++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8da3d39e3218..df411e776911 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1684,13 +1684,14 @@ static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
 static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 				    const struct switchdev_obj_port_vlan *vlan)
 {
+	struct dsa_port *dp = &ds->ports[i];
 	struct mv88e6xxx_chip *chip = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	u8 member;
 	u16 vid;
 
-	if (!chip->info->max_vid)
+	if (!chip->info->max_vid || br_vlan_enabled(dp->bridge_dev))
 		return;
 
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
@@ -1751,12 +1752,16 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 				   const struct switchdev_obj_port_vlan *vlan)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
+	struct dsa_port *dp = &ds->ports[i];
 	u16 pvid, vid;
 	int err = 0;
 
 	if (!chip->info->max_vid)
 		return -EOPNOTSUPP;
 
+	if (br_vlan_enabled(dp->bridge_dev))
+		return 0;
+
 	mutex_lock(&chip->reg_lock);
 
 	err = mv88e6xxx_port_get_pvid(chip, port, &pvid);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed0595459df1..111d7cfc8982 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -255,10 +255,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 	if (netif_is_bridge_master(vlan->obj.orig_dev))
 		return -EOPNOTSUPP;
 
-	if (br_vlan_enabled(dp->bridge_dev))
-		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
-
-	return 0;
+	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
 }
 
 int dsa_port_vlan_del(struct dsa_port *dp,
@@ -273,10 +270,7 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 	if (netif_is_bridge_master(vlan->obj.orig_dev))
 		return -EOPNOTSUPP;
 
-	if (br_vlan_enabled(dp->bridge_dev))
-		return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
-
-	return 0;
+	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
 }
 
 static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
-- 
2.17.1