Re: [PATCH net 2/5] net/sched: act_mpls: reject short linear Ethernet headers after pop
flat view
From: Eric Dumazet <edumazet@kernel.org>
Date: 2026-10-22 17:52:31
Also in:
stable
Le jeu. 22 oct. 2026 à 11:22, Jamal Hadi Salim [off-list ref] a écrit :
The issue: act_mpls POP can leave fewer than ETH_HLEN linear bytes after skb_mpls_pop(), either standalone on an Ethernet device or chained after act_vlan POP_ETH. A subsequent eth_type_trans() can then BUG in __skb_pull(). The Fix: After the pop, when the skb started as MPLS, drop it if the linear head is shorter than ETH_HLEN. Non-MPLS pops and pushes are unaffected. Conditions to recreate the bug: - egress tc filter on lo: action mpls pop (standalone), or action vlan pop_eth action mpls pop (chained). - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear eth / mpls / inner-eth frame. - The pop leaves skb_headlen(skb) < ETH_HLEN; a following eth_type_trans() can BUG in __skb_pull().
skb_mpls_pop() calls skb_ensure_writable(skb, mac_len + MPLS_HLEN), then memmove()s the mac header and pulls MPLS_HLEN. So after the pop we still have at least mac_len bytes in the linear part. If mac_len >= ETH_HLEN, the new check can never trigger. It only matters when mac_len < ETH_HLEN, ie after POP_ETH or on an L3 device. Do you have a standalone repro? If yes, which path ends up with mac_len < ETH_HLEN on an Ethernet device?
Fixes: 040b5cfbcefa ("Fixed updating of ethertype in function skb_mpls_pop")
This commit only added the @ethernet parameter to fix the ethertype update.
The pop leaving a short linear part dates back to act_mpls introduction:
Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
quoted hunk ↗ jump to hunk
Reported-by: Vega <redacted> Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/ (local) Cc: stable@vger.kernel.org Suggested-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> --- net/sched/act_mpls.c | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c index 4ea8b2e08c3a..4d375033400b 100644 --- a/net/sched/act_mpls.c +++ b/net/sched/act_mpls.c@@ -57,6 +57,7 @@ TC_INDIRECT_SCOPE int tcf_mpls_act(struct sk_buff *skb, struct tcf_mpls *m = to_mpls(a); struct tcf_mpls_params *p; __be32 new_lse; + bool was_mpls; int mac_len; tcf_lastuse_update(&m->tcf_tm);@@ -76,9 +77,12 @@ TC_INDIRECT_SCOPE int tcf_mpls_act(struct sk_buff *skb, switch (p->tcfm_action) { case TCA_MPLS_ACT_POP: + was_mpls = eth_p_mpls(skb->protocol); if (skb_mpls_pop(skb, p->tcfm_proto, mac_len, skb->dev && skb->dev->type == ARPHRD_ETHER)) goto drop; + if (was_mpls && unlikely(skb_headlen(skb) < ETH_HLEN)) + goto drop;
Same remark as Willem's on patch 1: please pull instead of dropping.
skb_mpls_pop() only guarantees the popped label was in the linear part.
On an L3 device (mac_len == 0) with a label stack, the next label and
the following headers can legitimately sit in a frag.
These packets work today, and this patch would silently drop them.
Something like:
if (mac_len < ETH_HLEN && !pskb_may_pull(skb, ETH_HLEN))
goto drop;
This would also remove the need for @was_mpls: if the packet
was not MPLS, skb_mpls_pop() did not change anything, and pulling
only drops packets shorter than ETH_HLEN.
Thanks.
break;
case TCA_MPLS_ACT_PUSH:
new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
--
2.43.0