stmmac_rx_vlan() reads the VLAN header straight from skb->data and removes it with skb_pull(), assuming the header is always in the linear part of the skb. That is not true once non-linear skbs can reach it: a program loaded with BPF_F_XDP_HAS_FRAGS can pass up a fragmented frame whose linear head has been shortened to as little as ETH_HLEN by bpf_xdp_adjust_head(), and the unguarded skb_pull() then drives skb->len below skb->data_len, tripping BUG() in __skb_pull(). Pull the VLAN header into the linear part with pskb_may_pull() before touching it and re-derive the header pointer afterwards, as pskb_may_pull() may reallocate the skb head. 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. Return an error so the callers can drop the frame when the header cannot be pulled. Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy") Signed-off-by: Lorenzo Bianconi --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 45 +++++++++++++++-------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index e929cc0f2312..04dc6dcaa435 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5095,22 +5095,30 @@ 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, VLAN_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; + u16 vlanid = ntohs(veth->h_vlan_TCI); + u8 meta_len = skb_metadata_len(skb); + /* pop the vlan tag */ - 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, @@ -5530,12 +5538,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)) @@ -6041,12 +6052,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