Netdev Archive on
help / color / mirror / Atom feed
From: Vladimir Oltean <>
To:, Jakub Kicinski <>,
	"David S. Miller" <>
Cc: Andrew Lunn <>,
	Florian Fainelli <>,
	Vivien Didelot <>
Subject: [RFC PATCH net-next 07/10] net: dsa: sja1105: prevent tag_8021q VLANs from being received on user ports
Date: Sat, 31 Jul 2021 03:14:05 +0300	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Currently it is possible for an attacker to craft packets with a fake
DSA tag and send them to us, and our user ports will accept them and
preserve that VLAN when transmitting towards the CPU. Then the tagger
will be misguided into thinking that the packets came on a different
port than they really came on.

Up until recently there wasn't a good option to prevent this from
happening. In SJA1105P and later, the MAC Configuration Table introduced
two options called:
- DRPSITAG: Drop Single Inner Tagged Frames
- DRPSOTAG: Drop Single Outer Tagged Frames

Because the sja1105 driver classifies all VLANs as "outer VLANs" (S-Tags),
it would be in principle possible to enable the DRPSOTAG bit on ports
using tag_8021q, and drop on ingress all packets which have a VLAN tag.
When the switch is VLAN-unaware, this works, because it uses a custom
TPID of 0xdadb, so any "tagged" packets received on a user port are
probably a spoofing attempt. But when the switch overall is VLAN-aware,
and some ports are standalone (therefore they use tag_8021q), the TPID
is 0x8100, and the port can receive a mix of untagged and VLAN-tagged
packets. The untagged ones will be classified to the tag_8021q pvid, and
the tagged ones to the VLAN ID from the packet header. Yes, it is true
that since commit 4fbc08bd3665 ("net: dsa: sja1105: deny 8021q uppers on
ports") we no longer support this mixed mode, but that is a temporary
limitation which will eventually be lifted. It would be nice to not
introduce one more restriction via DRPSOTAG, which would make the
standalone ports of a VLAN-aware switch drop genuinely VLAN-tagged

Also, the DRPSOTAG bit is not available on the first generation of
switches (SJA1105E, SJA1105T). So since one of the key features of this
driver is compatibility across switch generations, this makes it an even
less desirable approach.

The revelation comes from commit "net: dsa: sja1105: make sure untagged
packets are dropped on ingress ports with no pvid", where it became
obvious that untagged packets are not dropped even if the ingress port
is not in the VMEMB_PORT vector of that port's pvid. However, VLAN-tagged
packets are subject to VLAN ingress checking/dropping. This means that
instead of using the catch-all DRPSOTAG bit introduced in SJA1105P, we
can drop tagged packets on a per-VLAN basis, and this is already
compatible with SJA1105E/T.

This patch adds an "allowed_ingress" argument to sja1105_vlan_add(), and
we call it with "false" for tag_8021q VLANs on user ports. The tag_8021q
VLANs still need to be allowed, of course, on ingress to DSA ports and
CPU ports.

We also need to refine the drop_untagged check in sja1105_commit_pvid to
make it not freak out about this new configuration. Currently it will
try to keep the configuration consistent between untagged and pvid-tagged
packets, so if the pvid of a port is 1 but VLAN 1 is not in VMEMB_PORT,
packets tagged with VID 1 will behave the same as untagged packets, and
be dropped. This behavior is what we want for ports under a VLAN-aware
bridge, but for the ports with a tag_8021q pvid, we want untagged
packets to be accepted, but packets tagged with a header recognized by
the switch as a tag_8021q VLAN to be dropped. So only restrict the
drop_untagged check to apply to the bridge_pvid, not to the tag_8021q_pvid.

Signed-off-by: Vladimir Oltean <>
 drivers/net/dsa/sja1105/sja1105_main.c | 37 ++++++++++++++++++++------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index ff6f08037234..31dd4b5a2c80 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -122,12 +122,21 @@ static int sja1105_commit_pvid(struct dsa_switch *ds, int port)
 	if (rc)
 		return rc;
-	vlan = priv->static_config.tables[BLK_IDX_VLAN_LOOKUP].entries;
+	/* Only force dropping of untagged packets when the port is under a
+	 * VLAN-aware bridge. When the tag_8021q pvid is used, we are
+	 * deliberately removing the RX VLAN from the port's VMEMB_PORT list,
+	 * to prevent DSA tag spoofing from the link partner. Untagged packets
+	 * are the only ones that should be received with tag_8021q, so
+	 * definitely don't drop them.
+	 */
+	if (pvid == priv->bridge_pvid[port]) {
+		vlan = priv->static_config.tables[BLK_IDX_VLAN_LOOKUP].entries;
-	match = sja1105_is_vlan_configured(priv, pvid);
+		match = sja1105_is_vlan_configured(priv, pvid);
-	if (match < 0 || !(vlan[match].vmemb_port & BIT(port)))
-		drop_untagged = true;
+		if (match < 0 || !(vlan[match].vmemb_port & BIT(port)))
+			drop_untagged = true;
+	}
 	return sja1105_drop_untagged(ds, port, drop_untagged);
@@ -2249,7 +2258,7 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 static int sja1105_vlan_add(struct sja1105_private *priv, int port, u16 vid,
-			    u16 flags)
+			    u16 flags, bool allowed_ingress)
 	struct sja1105_vlan_lookup_entry *vlan;
 	struct sja1105_table *table;
@@ -2271,7 +2280,12 @@ static int sja1105_vlan_add(struct sja1105_private *priv, int port, u16 vid,
 	vlan[match].type_entry = SJA1110_VLAN_D_TAG;
 	vlan[match].vlanid = vid;
 	vlan[match].vlan_bc |= BIT(port);
-	vlan[match].vmemb_port |= BIT(port);
+	if (allowed_ingress)
+		vlan[match].vmemb_port |= BIT(port);
+	else
+		vlan[match].vmemb_port &= ~BIT(port);
 		vlan[match].tag_port &= ~BIT(port);
@@ -2343,7 +2357,7 @@ static int sja1105_bridge_vlan_add(struct dsa_switch *ds, int port,
 	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
 		flags = 0;
-	rc = sja1105_vlan_add(priv, port, vlan->vid, flags);
+	rc = sja1105_vlan_add(priv, port, vlan->vid, flags, true);
 	if (rc)
 		return rc;
@@ -2373,9 +2387,16 @@ static int sja1105_dsa_8021q_vlan_add(struct dsa_switch *ds, int port, u16 vid,
 				      u16 flags)
 	struct sja1105_private *priv = ds->priv;
+	bool allowed_ingress = true;
 	int rc;
-	rc = sja1105_vlan_add(priv, port, vid, flags);
+	/* Prevent attackers from trying to inject a DSA tag from
+	 * the outside world.
+	 */
+	if (dsa_is_user_port(ds, port))
+		allowed_ingress = false;
+	rc = sja1105_vlan_add(priv, port, vid, flags, allowed_ingress);
 	if (rc)
 		return rc;

  parent reply	other threads:[~2021-07-31  0:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-31  0:13 [RFC PATCH net-next 00/10] NXP SJA1105 driver support for "H" switch topologies Vladimir Oltean
2021-07-31  0:13 ` [RFC PATCH net-next 01/10] net: dsa: rename teardown_default_cpu to teardown_cpu_ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 02/10] net: dsa: give preference to local CPU ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 03/10] net: dsa: sja1105: configure the cascade ports based on topology Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 04/10] net: dsa: sja1105: manage the forwarding domain towards DSA ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 05/10] net: dsa: sja1105: manage VLANs on cascade ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 06/10] net: dsa: sja1105: suppress TX packets from looping back in "H" topologies Vladimir Oltean
2021-07-31  0:14 ` Vladimir Oltean [this message]
2021-07-31  0:14 ` [RFC PATCH net-next 08/10] net: dsa: sja1105: increase MTU to account for VLAN header on DSA ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 09/10] net: dsa: sja1105: enable address learning on cascade ports Vladimir Oltean
2021-07-31  0:14 ` [RFC PATCH net-next 10/10] net: dsa: sja1105: drop untagged packets on the CPU and DSA ports Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \
    --subject='Re: [RFC PATCH net-next 07/10] net: dsa: sja1105: prevent tag_8021q VLANs from being received on user ports' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).