6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Vladimir Oltean <vladimir.oltean@xxxxxxx>
[ Upstream commit 0bcf2e4aca6c29a07555b713f2fb461dc38d5977 ]
ocelot_xmit_get_vlan_info() calls __skb_vlan_pop() as the most
appropriate helper I could find which strips away a VLAN header.
That's all I need it to do, but __skb_vlan_pop() has more logic, which
will become incompatible with the future revert of commit 6d1ccff62780
("net: reset mac header in dev_start_xmit()").
Namely, it performs a sanity check on skb_mac_header(), which will stop
being set after the above revert, so it will return an error instead of
removing the VLAN tag.
ocelot_xmit_get_vlan_info() gets called in 2 circumstances:
(1) the port is under a VLAN-aware bridge and the bridge sends
VLAN-tagged packets
(2) the port is under a VLAN-aware bridge and somebody else (an 8021q
upper) sends VLAN-tagged packets (using a VID that isn't in the
bridge vlan tables)
In case (1), there is actually no bug to defend against, because
br_dev_xmit() calls skb_reset_mac_header() and things continue to work.
However, in case (2), illustrated using the commands below, it can be
seen that our intervention is needed, since __skb_vlan_pop() complains:
$ ip link add br0 type bridge vlan_filtering 1 && ip link set br0 up
$ ip link set $eth master br0 && ip link set $eth up
$ ip link add link $eth name $eth.100 type vlan id 100 && ip link set $eth.100 up
$ ip addr add 192.168.100.1/24 dev $eth.100
I could fend off the checks in __skb_vlan_pop() with some
skb_mac_header_was_set() calls, but seeing how few callers of
__skb_vlan_pop() there are from TX paths, that seems rather
unproductive.
As an alternative solution, extract the bare minimum logic to strip a
VLAN header, and move it to a new helper named vlan_remove_tag(), close
to the definition of vlan_insert_tag(). Document it appropriately and
make ocelot_xmit_get_vlan_info() call this smaller helper instead.
Seeing that it doesn't appear illegal to test skb->protocol in the TX
path, I guess it would be a good for vlan_remove_tag() to also absorb
the vlan_set_encap_proto() function call.
Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Stable-dep-of: 67c3ca2c5cfe ("net: mscc: ocelot: use ocelot_xmit_get_vlan_info() also for FDMA and register injection")
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
include/linux/if_vlan.h | 21 +++++++++++++++++++++
net/core/skbuff.c | 8 +-------
net/dsa/tag_ocelot.c | 2 +-
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index e0d0a645be7cf..83266201746c1 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -704,6 +704,27 @@ static inline void vlan_set_encap_proto(struct sk_buff *skb,
skb->protocol = htons(ETH_P_802_2);
}
+/**
+ * vlan_remove_tag - remove outer VLAN tag from payload
+ * @skb: skbuff to remove tag from
+ * @vlan_tci: buffer to store value
+ *
+ * Expects the skb to contain a VLAN tag in the payload, and to have skb->data
+ * pointing at the MAC header.
+ *
+ * Returns a new pointer to skb->data, or NULL on failure to pull.
+ */
+static inline void *vlan_remove_tag(struct sk_buff *skb, u16 *vlan_tci)
+{
+ struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
+
+ *vlan_tci = ntohs(vhdr->h_vlan_TCI);
+
+ memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
+ vlan_set_encap_proto(skb, vhdr);
+ return __skb_pull(skb, VLAN_HLEN);
+}
+
/**
* skb_vlan_tagged - check if skb is vlan tagged.
* @skb: skbuff to query
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4d46788cd493a..768b8d65a5baa 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5791,7 +5791,6 @@ EXPORT_SYMBOL(skb_ensure_writable);
*/
int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
{
- struct vlan_hdr *vhdr;
int offset = skb->data - skb_mac_header(skb);
int err;
@@ -5807,13 +5806,8 @@ int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
- vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
- *vlan_tci = ntohs(vhdr->h_vlan_TCI);
-
- memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
- __skb_pull(skb, VLAN_HLEN);
+ vlan_remove_tag(skb, vlan_tci);
- vlan_set_encap_proto(skb, vhdr);
skb->mac_header += VLAN_HLEN;
if (skb_network_offset(skb) < ETH_HLEN)
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index afca3cdf190a0..18dda9423fae5 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -26,7 +26,7 @@ static void ocelot_xmit_get_vlan_info(struct sk_buff *skb, struct dsa_port *dp,
br_vlan_get_proto(br, &proto);
if (ntohs(hdr->h_vlan_proto) == proto) {
- __skb_vlan_pop(skb, &tci);
+ vlan_remove_tag(skb, &tci);
*vlan_tci = tci;
} else {
rcu_read_lock();
--
2.43.0
