stmmac_rx_vlan() reads the VLAN header straight from skb->data and removes it with skb_pull(), assuming the header is fully present in the linear part of the skb. XDP can change the frame length and head layout before the tag is stripped. An XDP program can use bpf_xdp_adjust_tail() to shrink a frame to as little as ETH_HLEN while bytes 12-13 still contain a VLAN ethertype. The driver then reads h_vlan_TCI past skb->len and pops the tag, leaving less than ETH_HLEN in the skb. eth_type_trans() cannot pull the MAC header, so a malformed skb is passed up. The TCI is also read from the wrong place when the tag is not fully in the linear head, e.g. when it spans into a frag. Read h_vlan_proto after pulling ETH_HLEN into the linear part, and check it together with the device features. Only if the tag is actually going to be popped, pull VLAN_ETH_HLEN and re-derive the header pointer afterwards, as pskb_may_pull() may reallocate the skb head. Return an error so the callers can drop a frame that claims a VLAN tag but cannot provide the full header; untagged frames, including short ones, are left untouched. Also move the XDP metadata written with bpf_xdp_adjust_meta() together with the MAC header when the tag is popped, so data_meta still points at the metadata after eth_type_trans() anchors mac_header, instead of at the first bytes of the destination MAC. Fixes: 5fabb01207a2 ("net: stmmac: Add initial XDP support") Reviewed-by: Nicolai Buchwitz Signed-off-by: Lorenzo Bianconi --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 48 ++++++++++++++++------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 8a2faf50d695..32b2c5834c34 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5102,22 +5102,35 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb, return vlan_features_check(skb, features); } -static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb) +static int stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb) { - struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb); - __be16 vlan_proto = veth->h_vlan_proto; - u16 vlanid; + struct vlan_ethhdr *veth; - if ((vlan_proto == htons(ETH_P_8021Q) && + if (!pskb_may_pull(skb, ETH_HLEN)) + return -EINVAL; + + veth = skb_vlan_eth_hdr(skb); + if ((veth->h_vlan_proto == htons(ETH_P_8021Q) && dev->features & NETIF_F_HW_VLAN_CTAG_RX) || - (vlan_proto == htons(ETH_P_8021AD) && + (veth->h_vlan_proto == htons(ETH_P_8021AD) && dev->features & NETIF_F_HW_VLAN_STAG_RX)) { + __be16 vlan_proto = veth->h_vlan_proto; + u8 meta_len = skb_metadata_len(skb); + u16 vlanid; + /* pop the vlan tag */ + if (!pskb_may_pull(skb, VLAN_ETH_HLEN)) + return -EINVAL; + + veth = skb_vlan_eth_hdr(skb); vlanid = ntohs(veth->h_vlan_TCI); - memmove(skb->data + VLAN_HLEN, veth, ETH_ALEN * 2); + memmove(skb->data - meta_len + VLAN_HLEN, + skb->data - meta_len, meta_len + ETH_ALEN * 2); skb_pull(skb, VLAN_HLEN); __vlan_hwaccel_put_tag(skb, vlan_proto, vlanid); } + + return 0; } static bool stmmac_rx_need_ic(struct stmmac_priv *priv, @@ -5546,12 +5559,15 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue, } stmmac_get_rx_hwtstamp(priv, p, np, skb); - if (priv->hw->hw_vlan_en) + if (priv->hw->hw_vlan_en) { /* MAC level stripping. */ stmmac_rx_hw_vlan(priv, priv->hw, p, skb); - else - /* Driver level stripping. */ - stmmac_rx_vlan(priv->dev, skb); + } else if (stmmac_rx_vlan(priv->dev, skb)) { + dev_kfree_skb_any(skb); + priv->xstats.rx_dropped++; + return; + } + skb->protocol = eth_type_trans(skb, priv->dev); if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb)) @@ -6057,12 +6073,14 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) stmmac_get_rx_hwtstamp(priv, p, np, skb); - if (priv->hw->hw_vlan_en) + if (priv->hw->hw_vlan_en) { /* MAC level stripping. */ stmmac_rx_hw_vlan(priv, priv->hw, p, skb); - else - /* Driver level stripping. */ - stmmac_rx_vlan(priv->dev, skb); + } else if (stmmac_rx_vlan(priv->dev, skb)) { + dev_kfree_skb_any(skb); + rx_dropped++; + goto next; + } skb->protocol = eth_type_trans(skb, priv->dev); -- 2.56.0