[PATCH net v3 0/6] net: pull short linear Ethernet headers before eth_type_trans()

[PATCH net v3 0/6] net: pull short linear Ethernet headers before eth_type_trans()

[Jamal Hadi Salim]

This series fixes a class of producer/consumer bugs in which an outer
header is popped and the packet is then handed to a consumer with fewer
than ETH_HLEN bytes in the linear area.  eth_type_trans() pulls ETH_HLEN
from that area and can BUG in __skb_pull() when the inner Ethernet header
is not linear.  The affected producers are act_vlan POP_ETH, act_mpls POP,
act_ife egress decode, and the Open vSwitch MPLS/TEB and NSH pops.  A
consumer-side guard in act_mirred ensures a pullable link-layer header at
the redirect boundary.

Patch 1 makes act_vlan POP_ETH pull the inner Ethernet header after the
pop and drop only when the packet is genuinely shorter than ETH_HLEN.
Patch 2 does the same for act_mpls when the configured inner protocol is
ETH_P_TEB.  Other inner protocols are not required to carry an Ethernet
header.  Patch 3 ensures a pullable link-layer header at the redirect
boundary.  Patch 4 keeps act_ife egress
decode from consuming the inner Ethernet header and restores the header
metadata.

Patches 5 and 6 establish the same linear-header invariant for the OVS
MPLS/TEB and NSH pops before restoring MAC_PROTO_ETHERNET.

Changes in v3:
  - act_vlan: keep the unconditional post-pop pskb_may_pull(ETH_HLEN).
    After discussion with Willem I considered vetting it on ETH_P_TEB,
    but was hesitant because i felt checking TEB in act_vlan was
    in bad taste: POP_ETH has no inner-protocol parameter. Eyeballing
    also showed the TEB test is not needed for the eth_type_trans()
    failure this series fixes (of course sashikos will have something
    to say;->).
  - act_mpls: drop the pre-pop mac_len gate and pull ETH_HLEN only for
    ETH_P_TEB.  Other act_mpls inner protocols are not required to carry
    an Ethernet header (Willem de Bruijn).
  - act_mirred: drop the skb_network_offset() condition and keep only
    pskb_may_pull(dev->min_header_len).  The offset check rejected valid
    MPLS L2VPN redirects (Eric Dumazet; tc_mpls_l2vpn.sh).
  - act_vlan and act_ife: no code change; add Eric Dumazet's Reviewed-by.
  - OVS TEB and NSH: no code change; add Aaron Conole's Reviewed-by.

Changes in v2:
  - Pull the inner header with pskb_may_pull(skb, ETH_HLEN) and drop only
    on failure, instead of dropping on the post-pop skb_headlen() check
    which may drop frags (Eric/Willem).
  - act_mpls: gate the pull on the pre-pop mac_len and remove the separate
    was_mpls state.
  - Drop the v1 TUN patch; add the act_mirred consumer-side guard.
  - Add the act_ife egress decode fix; decode is receive-side, and the
    egress path restores the frame for loopback-style transmit paths.
  - Keep the OVS TEB and NSH patches unchanged and restore their v1
    Reviewed-by trailers.
  - Drop two bouncing Cc addresses.

Jamal Hadi Salim (6):
  net/sched: act_vlan: pull the inner Ethernet header after POP_ETH
  net/sched: act_mpls: pull the inner Ethernet header after POP
  net/sched: act_mirred: ensure a pullable link-layer header before redirect
  net/sched: act_ife: do not consume the inner header on egress
  net: openvswitch: ensure a linear inner Ethernet header on TEB pop
  net: openvswitch: ensure a linear inner Ethernet header on NSH pop

 net/openvswitch/actions.c | 12 +++++++++---
 net/sched/act_ife.c       |  4 ++++
 net/sched/act_mirred.c    |  6 ++++++
 net/sched/act_mpls.c      |  3 +++
 net/sched/act_vlan.c      |  2 ++
 5 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH net v3 1/6] net/sched: act_vlan: pull the inner Ethernet header after POP_ETH

[Jamal Hadi Salim]

The issue:
act_vlan POP_ETH removes the outer Ethernet header and leaves the inner
Ethernet header at the head of the packet.  A non-linear skb can have the
inner header in a page fragment, so a following eth_type_trans() may find
less than ETH_HLEN bytes in the linear area and BUG in __skb_pull().

The Fix:
Pull the inner Ethernet header into the linear area after skb_eth_pop():
drop the packet only when pskb_may_pull(skb, ETH_HLEN) fails.  This keeps
valid non-linear packets whose inner header sits in frags.

Conditions to recreate the bug:
- egress tc filter on lo: action vlan pop_eth.
- AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
  eth / inner-eth frame.
- The inner header is in page frags; a following eth_type_trans() can BUG
  in __skb_pull().

Fixes: 19fbcb36a39e ("net/sched: act_vlan: Add {POP,PUSH}_ETH actions")
Reported-by: Vega <vega@nebusec.ai>
Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.38722ee638a21@gmail.com/
Link: https://lore.kernel.org/netdev/CAL4WiipJLzZ9ywOPaFHZ+wePTOuO+6+aaRCVXMHz+xmmQm2hcA@mail.gmail.com/
Cc: stable@vger.kernel.org
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - no code change; add Eric Dumazet's Reviewed-by.  act_mpls makes its
    pull unconditional and act_mirred drops the skb_network_offset()
    condition; see those patches.

v1 -> v2:
  - pull the inner Ethernet header with pskb_may_pull() and drop only
    when the packet is too short;
  - keep the OVS TEB and NSH members and restore their Reviewed-by
    trailers;
  - replace the TUN patch with the act_mirred consumer guard and add
    the act_ife egress fix.
---
 net/sched/act_vlan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 6a375fdc63dd..bca95e0bbf64 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -79,6 +79,8 @@ TC_INDIRECT_SCOPE int tcf_vlan_act(struct sk_buff *skb,
 		err = skb_eth_pop(skb);
 		if (err)
 			goto drop;
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			goto drop;
 		break;
 	case TCA_VLAN_ACT_PUSH_ETH:
 		err = skb_eth_push(skb, p->tcfv_push_dst, p->tcfv_push_src);
-- 
2.43.0

[PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Jamal Hadi Salim]

The issue:
When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
the inner Ethernet header in a page fragment after skb_mpls_pop(), either
on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
following eth_type_trans() can then BUG in __skb_pull().

The Fix:
After the pop, pull the inner Ethernet header into the linear area only
when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
header and must not be required to carry one.  The pull is a no-op when
the header is already linear.

Conditions to recreate the bug:
- egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
  (chained), or on an L3 device where mac_len == 0.
- AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
  eth / mpls / inner-eth frame.
- The inner header is in page frags; a following eth_type_trans() can BUG
  in __skb_pull().

Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
Reported-by: Vega <vega@nebusec.ai>
Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
Cc: stable@vger.kernel.org
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - drop the pre-pop mac_len gate.  The pull is a no-op when the header
    is already linear, so the gate only avoided a cheap call (Willem de
    Bruijn).
  - restrict the pull to ETH_P_TEB.  act_mpls POP can target IP, MPLS,
    etc.; those inner protocols have no Ethernet header and must not be
    required to carry one (Willem de Bruijn).
---
 net/sched/act_mpls.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
index 4ea8b2e08c3a..35a6d27f76a4 100644
--- a/net/sched/act_mpls.c
+++ b/net/sched/act_mpls.c
@@ -79,6 +79,9 @@
 		if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
 				 skb->dev && skb->dev->type == ARPHRD_ETHER))
 			goto drop;
+		if (p->tcfm_proto == htons(ETH_P_TEB) &&
+		    !pskb_may_pull(skb, ETH_HLEN))
+			goto drop;
 		break;
 	case TCA_MPLS_ACT_PUSH:
 		new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
-- 
2.43.0

[PATCH net v3 3/6] net/sched: act_mirred: ensure a pullable link-layer header before redirect

[Jamal Hadi Salim]

The issue:
tcf_mirred_to_dev() may redirect a packet that has no link-layer header to
a device that expects one.  TUN builds a valid L3 skb with mac_len == 0;
mirred then pushes zero bytes and the IP header is parsed as Ethernet by
eth_type_trans().  The same happens after a VLAN POP_ETH or MPLS POP chain
whose producer did not leave a full inner Ethernet header.

The Fix:
After the existing push/pull adjustment, when the target expects data at
the MAC header, pull dev->min_header_len bytes into the linear area and
drop the packet only when that pull fails.  This ensures that a consumer
such as eth_type_trans() can pull the target device's minimum link-layer
header from the linear area.  Do not gate the check on skb_network_offset():
tc_mpls_l2vpn.sh legitimately redirects a decapsulated packet whose MAC
header is not at the recorded network offset.

Conditions to recreate the bug:
- create a TUN device with IFF_TUN | IFF_VNET_HDR | IFF_NO_PI;
- attach a filter to the TUN device's ingress hook, with a mirred egress
  redirect action targeting an Ethernet device such as lo;
- write a virtio_net_hdr with hdr_len=1 and an 8192-byte payload.
The unfixed kernel parses the IP header as Ethernet and can BUG in
eth_type_trans() via loopback_xmit.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Link: https://lore.kernel.org/netdev/CAL4Wiiq3h_OoVo9BpBy0pqFe=AeyN25QxsU86h63qEf05uCJRA@mail.gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - drop the skb_network_offset() condition; keep only
    pskb_may_pull(dev->min_header_len).  The offset check rejected valid
    MPLS L2VPN redirects (Eric Dumazet; tc_mpls_l2vpn.sh).
---
 net/sched/act_mirred.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 553342c55cf7..85163c46d652 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -316,6 +316,10 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
 		}
 	}
 
+	if (!expects_nh &&
+	    !pskb_may_pull(skb_to_send, dev->min_header_len))
+		goto err_cant_do;
+
 	skb_to_send->skb_iif = skb->dev->ifindex;
 	skb_to_send->dev = dev;
 
@@ -335,6 +339,8 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
 	return retval;
 
 err_cant_do:
+	if (skb_to_send != skb)
+		kfree_skb(skb_to_send);
 	if (is_redirect)
 		retval = TC_ACT_SHOT;
 	tcf_action_inc_overlimit_qstats(&m->common);
-- 
2.43.0

[PATCH net v3 4/6] net/sched: act_ife: do not consume the inner header on egress

[Jamal Hadi Salim]

The issue:
tcf_ife_decode() unconditionally calls eth_type_trans(), which pulls
ETH_HLEN.  On egress this consumes the inner Ethernet header; a following
eth_type_trans() from loopback_xmit() then parses L3 data as Ethernet and
can BUG in __skb_pull() when the remaining linear area is short.

The Fix:
Parse the inner Ethernet header with eth_type_trans() in both directions,
then restore the egress data position and MAC length.  On ingress the
existing behavior is unchanged.  On egress, push the inner Ethernet header
back after parsing so the xmit path sees the original frame and later
actions see a consistent network header.

Decode is a receive-side operation; the egress branch keeps the frame
intact for transmit paths such as loopback.  tcf_ife_is_eth_skb() accepts
Ethernet and loopback devices generally.

Conditions to recreate the bug:
- egress tc filter on lo: action ife decode pipe;
- AF_PACKET SOCK_RAW frame: outer Ethernet with ETH_P_IFE, IFE metadata,
  inner Ethernet and an 8192-byte payload;
- the egress pipe re-enters eth_type_trans() via loopback_xmit() after the
  action consumed the inner header.

Fixes: ef6980b6becb ("introduce IFE action")
Cc: stable@vger.kernel.org
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - no code change; add Eric Dumazet's Reviewed-by.
---
 net/sched/act_ife.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 2afd68983ece..3946a9589bb8 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -769,6 +769,10 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 
 	skb->protocol = eth_type_trans(skb, skb->dev);
 	skb_reset_network_header(skb);
+	if (!skb_at_tc_ingress(skb)) {
+		skb_push(skb, ETH_HLEN);
+		skb_reset_mac_len(skb);
+	}
 
 	return action;
 }
-- 
2.43.0

[PATCH net v3 5/6] net: openvswitch: ensure a linear inner Ethernet header on TEB pop

[Jamal Hadi Salim]

The issue:
pop_mpls() restores MAC_PROTO_ETHERNET when the popped ethertype is
ETH_P_TEB, but skb_mpls_pop() only guarantees that the MPLS shim is in
the linear area.  When the inner Ethernet header sits in a page
fragment, the datapath now claims an Ethernet header it cannot access
linearly.

Output to an OVS internal vport then calls eth_type_trans(), which
pulls ETH_HLEN from an empty linear area and triggers the BUG in
__skb_pull():

  kernel BUG at ./include/linux/skbuff.h:2830!
  RIP: 0010:eth_type_trans+0x335/0x500
  internal_dev_recv
  ovs_vport_send
  do_output
  do_execute_actions
  ovs_execute_actions
  ovs_dp_process_packet
  ovs_vport_receive
  internal_dev_xmit

Conditions to recreate the bug:
On a system with 4 KiB pages and CONFIG_OPENVSWITCH and CONFIG_PACKET
enabled, use CAP_NET_ADMIN to create an OVS datapath and bring its local
internal port up with MTU 9000.  Install a flow matching an Ethernet/MPLS
packet with one bottom-of-stack label and actions pop_eth,
pop_mpls(0x6558), output to the local port.  With CAP_NET_RAW, send a
4128-byte AF_PACKET/SOCK_RAW frame containing outer Ethernet, the MPLS
label, inner Ethernet and 4096 payload bytes.  The two pops consume the
linear headers and expose the inner Ethernet header in page fragments.

The Fix:
Pull the inner Ethernet header into the linear area before restoring
MAC_PROTO_ETHERNET.

Fixes: f66b53fdbb22 ("openvswitch: New MPLS actions for layer 2 tunnelling")
Link: https://lore.kernel.org/netdev/e85fa16e-df3c-43dd-bb3a-9edca07200ea@kernel.org/
Cc: stable@vger.kernel.org
Reviewed-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - no code change; add Aaron Conole's Reviewed-by.
---
 net/openvswitch/actions.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 27152bca8a8c..03b31e33e880 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -153,8 +153,11 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	if (err)
 		return err;
 
-	if (ethertype == htons(ETH_P_TEB))
+	if (ethertype == htons(ETH_P_TEB)) {
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			return -ENOMEM;
 		key->mac_proto = MAC_PROTO_ETHERNET;
+	}
 
 	invalidate_flow_key(key);
 	return 0;
-- 
2.43.0

[PATCH net v3 6/6] net: openvswitch: ensure a linear inner Ethernet header on NSH pop

[Jamal Hadi Salim]

The issue:
pop_nsh() restores MAC_PROTO_ETHERNET when the popped NSH next-protocol
is TUN_P_ETHERNET, but nsh_pop() only guarantees that the NSH header is
in the linear area.  When the inner Ethernet header sits in a page
fragment, the datapath then claims an Ethernet header it cannot access
linearly.

Output to an OVS internal vport then calls eth_type_trans(), which
pulls ETH_HLEN from an empty linear area and triggers the BUG in
__skb_pull():

  kernel BUG at ./include/linux/skbuff.h:2830!
  RIP: 0010:eth_type_trans+0x335/0x500
  internal_dev_recv
  ovs_vport_send
  do_output
  do_execute_actions
  ovs_execute_actions
  ovs_dp_process_packet
  ovs_vport_receive
  internal_dev_xmit

Conditions to recreate the bug:
On a system with 4 KiB pages and CONFIG_OPENVSWITCH and CONFIG_PACKET
enabled, use CAP_NET_ADMIN to create an OVS datapath and bring its local
internal port up with MTU 9000.  Install a flow matching an Ethernet/NSH
packet with a 24-byte MD1 header whose next-protocol is TUN_P_ETHERNET,
and actions pop_eth, pop_nsh, output to the local port.  With
CAP_NET_RAW, send a 4148-byte AF_PACKET/SOCK_RAW frame containing outer
Ethernet, the NSH header, inner Ethernet and 4096 payload bytes.  The
two pops consume the linear headers and expose the inner Ethernet header
in page fragments.

The fix:
Pull the inner Ethernet header into the linear area before restoring
MAC_PROTO_ETHERNET.  This is the NSH counterpart of the MPLS/TEB
producer fix in the previous patch.

Fixes: b2d0f5d5dc53 ("openvswitch: enable NSH support")
Link: https://lore.kernel.org/netdev/e85fa16e-df3c-43dd-bb3a-9edca07200ea@kernel.org/
Cc: stable@vger.kernel.org
Reviewed-by: Aaron Conole <aconole@redhat.com>
Reviewed-by: Jiri Benc <jbenc@redhat.com>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

---
v2 -> v3:
  - no code change; add Aaron Conole's Reviewed-by.
---
 net/openvswitch/actions.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 03b31e33e880..cb7e16c6a8ef 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -314,10 +314,13 @@ static int pop_nsh(struct sk_buff *skb, struct sw_flow_key *key)
 		return err;
 
 	/* safe right before invalidate_flow_key */
-	if (skb->protocol == htons(ETH_P_TEB))
+	if (skb->protocol == htons(ETH_P_TEB)) {
+		if (!pskb_may_pull(skb, ETH_HLEN))
+			return -ENOMEM;
 		key->mac_proto = MAC_PROTO_ETHERNET;
-	else
+	} else {
 		key->mac_proto = MAC_PROTO_NONE;
+	}
 	invalidate_flow_key(key);
 	return 0;
 }
-- 
2.43.0

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Jamal Hadi Salim]

On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> The issue:
> When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> following eth_type_trans() can then BUG in __skb_pull().
>
> The Fix:
> After the pop, pull the inner Ethernet header into the linear area only
> when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> header and must not be required to carry one.  The pull is a no-op when
> the header is already linear.
>
> Conditions to recreate the bug:
> - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
>   (chained), or on an L3 device where mac_len == 0.
> - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
>   eth / mpls / inner-eth frame.
> - The inner header is in page frags; a following eth_type_trans() can BUG
>   in __skb_pull().
>
> Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> Reported-by: Vega <vega@nebusec.ai>
> Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> Cc: stable@vger.kernel.org
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Suggested-by: Eric Dumazet <edumazet@kernel.org>
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>

For the series

pw-bot: cr

Eric, you are reading this? See the discussion with Willem.

cheers,
jamal


> ---
> v2 -> v3:
>   - drop the pre-pop mac_len gate.  The pull is a no-op when the header
>     is already linear, so the gate only avoided a cheap call (Willem de
>     Bruijn).
>   - restrict the pull to ETH_P_TEB.  act_mpls POP can target IP, MPLS,
>     etc.; those inner protocols have no Ethernet header and must not be
>     required to carry one (Willem de Bruijn).
> ---
>  net/sched/act_mpls.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> index 4ea8b2e08c3a..35a6d27f76a4 100644
> --- a/net/sched/act_mpls.c
> +++ b/net/sched/act_mpls.c
> @@ -79,6 +79,9 @@
>                 if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
>                                  skb->dev && skb->dev->type == ARPHRD_ETHER))
>                         goto drop;
> +               if (p->tcfm_proto == htons(ETH_P_TEB) &&
> +                   !pskb_may_pull(skb, ETH_HLEN))
> +                       goto drop;
>                 break;
>         case TCA_MPLS_ACT_PUSH:
>                 new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
> --
> 2.43.0

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Jamal Hadi Salim]

On Mon, Oct 26, 2026 at 5:42 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > The issue:
> > When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> > the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> > on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> > following eth_type_trans() can then BUG in __skb_pull().
> >
> > The Fix:
> > After the pop, pull the inner Ethernet header into the linear area only
> > when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> > fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> > header and must not be required to carry one.  The pull is a no-op when
> > the header is already linear.
> >
> > Conditions to recreate the bug:
> > - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
> >   (chained), or on an L3 device where mac_len == 0.
> > - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
> >   eth / mpls / inner-eth frame.
> > - The inner header is in page frags; a following eth_type_trans() can BUG
> >   in __skb_pull().
> >
> > Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> > Reported-by: Vega <vega@nebusec.ai>
> > Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> > Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> > Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> > Cc: stable@vger.kernel.org
> > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Suggested-by: Eric Dumazet <edumazet@kernel.org>
> > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >
>
> For the series
>
> pw-bot: cr
>
> Eric, you are reading this? See the discussion with Willem.
>

And to be clear, I am suggesting we fix both the producers and consumers.
Basically, as established, producers alone cannot protect against the
eBPF  privileged bpf_skb_adjust_room(..., BPF_ADJ_ROOM_MAC, ...) or
bpf_xdp_adjust_head() which can re-shrink the head after any producer
fix.
And then we dont have to do this ETH_P_TEB check here.

so i am officially withdrawing this until we reach some consensus.


cheers,
jamal

>
> > ---
> > v2 -> v3:
> >   - drop the pre-pop mac_len gate.  The pull is a no-op when the header
> >     is already linear, so the gate only avoided a cheap call (Willem de
> >     Bruijn).
> >   - restrict the pull to ETH_P_TEB.  act_mpls POP can target IP, MPLS,
> >     etc.; those inner protocols have no Ethernet header and must not be
> >     required to carry one (Willem de Bruijn).
> > ---
> >  net/sched/act_mpls.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> > index 4ea8b2e08c3a..35a6d27f76a4 100644
> > --- a/net/sched/act_mpls.c
> > +++ b/net/sched/act_mpls.c
> > @@ -79,6 +79,9 @@
> >                 if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
> >                                  skb->dev && skb->dev->type == ARPHRD_ETHER))
> >                         goto drop;
> > +               if (p->tcfm_proto == htons(ETH_P_TEB) &&
> > +                   !pskb_may_pull(skb, ETH_HLEN))
> > +                       goto drop;
> >                 break;
> >         case TCA_MPLS_ACT_PUSH:
> >                 new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
> > --
> > 2.43.0

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Eric Dumazet]

Le lun. 26 oct. 2026 à 11:32, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> On Mon, Oct 26, 2026 at 5:42 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > The issue:
> > > When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> > > the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> > > on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> > > following eth_type_trans() can then BUG in __skb_pull().
> > >
> > > The Fix:
> > > After the pop, pull the inner Ethernet header into the linear area only
> > > when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> > > fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> > > header and must not be required to carry one.  The pull is a no-op when
> > > the header is already linear.
> > >
> > > Conditions to recreate the bug:
> > > - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
> > >   (chained), or on an L3 device where mac_len == 0.
> > > - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
> > >   eth / mpls / inner-eth frame.
> > > - The inner header is in page frags; a following eth_type_trans() can BUG
> > >   in __skb_pull().
> > >
> > > Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> > > Reported-by: Vega <vega@nebusec.ai>
> > > Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> > > Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> > > Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Suggested-by: Eric Dumazet <edumazet@kernel.org>
> > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > >
> >
> > For the series
> >
> > pw-bot: cr
> >
> > Eric, you are reading this? See the discussion with Willem.
> >
>
> And to be clear, I am suggesting we fix both the producers and consumers.

I don't think we need to touch loopback / eth_type_trans() directly
(that's the per-driver cost I pushed back on for tun). There is a single
natural choke point: the egress fall-through in sch_handle_egress(),
right before the skb goes back to the driver's ndo_start_xmit(). A
device advertises how much link-layer header it needs via
dev->min_header_len, and that is exactly the amount eth_type_trans()
will pull. One check there covers act_vlan, act_mpls with any inner
protocol, act_ife, pedit, and BPF, now and in the future:

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a09e7869118059e92b53e00ed873dfc7485ffc3..ffc8c4c0dfa35fc6eee12bec799809b0e2476270
100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4757,6 +4757,16 @@ sch_handle_egress(struct sk_buff *skb, int
*ret, struct net_device *dev)
        }
        bpf_net_ctx_clear(bpf_net_ctx);

+       /* Egress actions and programs can pop or shrink the link-layer
+        * header. Drivers expect dev->min_header_len bytes in the linear
+        * area (e.g. eth_type_trans() in loopback_xmit()).
+        */
+       if (unlikely(!pskb_may_pull(skb, dev->min_header_len))) {
+               kfree_skb_reason(skb, SKB_DROP_REASON_PKT_TOO_SMALL);
+               *ret = NET_XMIT_DROP;
+               return NULL;
+       }
+
        return skb;
 }
 #else

It is cheap: only runs when an egress tc/tcx program is attached, it is
an inline compare, and pskb_may_pull() is a no-op when the header is
already linear. For non-Ethernet devices min_header_len is 0 so nothing
changes. It is the same dev->min_header_len gate your act_mirred patch
already uses, just at the other exit point.

I applied *only* this hunk on top of net (none of patches 1, 2) and ran
your three reproducers on lo egress under PANIC_ON_OOPS=y:

  1) vlan pop_eth + bpf skb_adjust_room(-1, BPF_ADJ_ROOM_MAC)
  2) vlan pop_eth + mpls pop protocol ip
  3) vlan pop_eth alone, non-linear inner eth (AF_PACKET+PACKET_VNET_HDR)

Unpatched: all three BUG in eth_type_trans() via loopback_xmit().
With the hunk above: all three are dropped cleanly, no BUG.

So for v4 I'd suggest:
  - drop patches 1 and 2 (the egress check subsumes them, and the
    ETH_P_TEB taste question goes away);
  - keep patch 3 (act_mirred) - that's the redirect boundary, a
    different exit;
  - keep patch 4 (act_ife) - it fixes the egress frame, not just the
    crash;
  - keep patches 5 and 6 (OVS) as-is - OVS actions can't run arbitrary
    BPF, those producer fixes are correct and already reviewed, and they
    can go in independently.

One gap this does not cover: the BPF redirect path
(__bpf_redirect_common() in net/core/filter.c) only checks
mac_header < network_header, so a 1-byte "mac header" still reaches
eth_type_trans() via __dev_forward_skb2()/loopback_xmit(). I haven't
reproduced it yet but it looks like the same bug and the same
dev->min_header_len guard would close it. Can look into that separately.

Thanks,

>
>
> cheers,
> jamal
>
> >
> > > ---
> > > v2 -> v3:
> > >   - drop the pre-pop mac_len gate.  The pull is a no-op when the header
> > >     is already linear, so the gate only avoided a cheap call (Willem de
> > >     Bruijn).
> > >   - restrict the pull to ETH_P_TEB.  act_mpls POP can target IP, MPLS,
> > >     etc.; those inner protocols have no Ethernet header and must not be
> > >     required to carry one (Willem de Bruijn).
> > > ---
> > >  net/sched/act_mpls.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> > > index 4ea8b2e08c3a..35a6d27f76a4 100644
> > > --- a/net/sched/act_mpls.c
> > > +++ b/net/sched/act_mpls.c
> > > @@ -79,6 +79,9 @@
> > >                 if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
> > >                                  skb->dev && skb->dev->type == ARPHRD_ETHER))
> > >                         goto drop;
> > > +               if (p->tcfm_proto == htons(ETH_P_TEB) &&
> > > +                   !pskb_may_pull(skb, ETH_HLEN))
> > > +                       goto drop;
> > >                 break;
> > >         case TCA_MPLS_ACT_PUSH:
> > >                 new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
> > > --
> > > 2.43.0

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Jamal Hadi Salim]

On Mon, Oct 26, 2026 at 9:10 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le lun. 26 oct. 2026 à 11:32, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> >
> > On Mon, Oct 26, 2026 at 5:42 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > >
> > > On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > >
> > > > The issue:
> > > > When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> > > > the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> > > > on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> > > > following eth_type_trans() can then BUG in __skb_pull().
> > > >
> > > > The Fix:
> > > > After the pop, pull the inner Ethernet header into the linear area only
> > > > when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> > > > fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> > > > header and must not be required to carry one.  The pull is a no-op when
> > > > the header is already linear.
> > > >
> > > > Conditions to recreate the bug:
> > > > - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
> > > >   (chained), or on an L3 device where mac_len == 0.
> > > > - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
> > > >   eth / mpls / inner-eth frame.
> > > > - The inner header is in page frags; a following eth_type_trans() can BUG
> > > >   in __skb_pull().
> > > >
> > > > Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> > > > Reported-by: Vega <vega@nebusec.ai>
> > > > Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> > > > Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> > > > Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> > > > Cc: stable@vger.kernel.org
> > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > > Suggested-by: Eric Dumazet <edumazet@kernel.org>
> > > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > > >
> > >
> > > For the series
> > >
> > > pw-bot: cr
> > >
> > > Eric, you are reading this? See the discussion with Willem.
> > >
> >
> > And to be clear, I am suggesting we fix both the producers and consumers.
>
> I don't think we need to touch loopback / eth_type_trans() directly
> (that's the per-driver cost I pushed back on for tun). There is a single
> natural choke point: the egress fall-through in sch_handle_egress(),
> right before the skb goes back to the driver's ndo_start_xmit(). A
> device advertises how much link-layer header it needs via
> dev->min_header_len, and that is exactly the amount eth_type_trans()
> will pull. One check there covers act_vlan, act_mpls with any inner
> protocol, act_ife, pedit, and BPF, now and in the future:
>

You are resolving part of it with this choke point but: the primary
concern is that ebpf can make changes that will still cause crashes at
the consumer.
Point is not whether you guard against bpf_skb_adjust_room() but more
the boundary coverage i.e not correctness of the check. You are
suggesting to (iiuc) to fix the other stuff inside ebpf? Here are some
sample holes (there are probably others):

1) bpf_skb_adjust_room() then bpf_redirect() - note: bpf_redirect()
returns TC_ACT_REDIRECT; sch_handle_egress() returns at the redirect
case before the check
2) bpf_skb_adjust_room() then bpf_clone_redirect(); The clone is sent
immediately inside the helper and escapes; the original skb may then
return TC_ACT_OK and be checked
3) you are handlin egress only - ingress still vulnerable

and xdp totally bypasses sch_handle_egress() [think
bpf_xdp_adjust_head() then XDP_PASS or XDP redirect to veth

I havent looked closely at the OVS ones you said to drop but hand wave
review seems they can be bypassed (likely i am missing something), so
we cant drop those.

summary?
The concern “producers alone cannot protect” remains valid unless you
convince other wise ;-> Your turn...

cheers,
jamal

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1a09e7869118059e92b53e00ed873dfc7485ffc3..ffc8c4c0dfa35fc6eee12bec799809b0e2476270
> 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4757,6 +4757,16 @@ sch_handle_egress(struct sk_buff *skb, int
> *ret, struct net_device *dev)
>         }
>         bpf_net_ctx_clear(bpf_net_ctx);
>
> +       /* Egress actions and programs can pop or shrink the link-layer
> +        * header. Drivers expect dev->min_header_len bytes in the linear
> +        * area (e.g. eth_type_trans() in loopback_xmit()).
> +        */
> +       if (unlikely(!pskb_may_pull(skb, dev->min_header_len))) {
> +               kfree_skb_reason(skb, SKB_DROP_REASON_PKT_TOO_SMALL);
> +               *ret = NET_XMIT_DROP;
> +               return NULL;
> +       }
> +
>         return skb;
>  }
>  #else
>
> It is cheap: only runs when an egress tc/tcx program is attached, it is
> an inline compare, and pskb_may_pull() is a no-op when the header is
> already linear. For non-Ethernet devices min_header_len is 0 so nothing
> changes. It is the same dev->min_header_len gate your act_mirred patch
> already uses, just at the other exit point.
>
> I applied *only* this hunk on top of net (none of patches 1, 2) and ran
> your three reproducers on lo egress under PANIC_ON_OOPS=y:
>
>   1) vlan pop_eth + bpf skb_adjust_room(-1, BPF_ADJ_ROOM_MAC)
>   2) vlan pop_eth + mpls pop protocol ip
>   3) vlan pop_eth alone, non-linear inner eth (AF_PACKET+PACKET_VNET_HDR)
>
> Unpatched: all three BUG in eth_type_trans() via loopback_xmit().
> With the hunk above: all three are dropped cleanly, no BUG.
>
> So for v4 I'd suggest:
>   - drop patches 1 and 2 (the egress check subsumes them, and the
>     ETH_P_TEB taste question goes away);
>   - keep patch 3 (act_mirred) - that's the redirect boundary, a
>     different exit;
>   - keep patch 4 (act_ife) - it fixes the egress frame, not just the
>     crash;
>   - keep patches 5 and 6 (OVS) as-is - OVS actions can't run arbitrary
>     BPF, those producer fixes are correct and already reviewed, and they
>     can go in independently.
>
> One gap this does not cover: the BPF redirect path
> (__bpf_redirect_common() in net/core/filter.c) only checks
> mac_header < network_header, so a 1-byte "mac header" still reaches
> eth_type_trans() via __dev_forward_skb2()/loopback_xmit(). I haven't
> reproduced it yet but it looks like the same bug and the same
> dev->min_header_len guard would close it. Can look into that separately.
>
> Thanks,
>
> >
> >
> > cheers,
> > jamal
> >
> > >
> > > > ---
> > > > v2 -> v3:
> > > >   - drop the pre-pop mac_len gate.  The pull is a no-op when the header
> > > >     is already linear, so the gate only avoided a cheap call (Willem de
> > > >     Bruijn).
> > > >   - restrict the pull to ETH_P_TEB.  act_mpls POP can target IP, MPLS,
> > > >     etc.; those inner protocols have no Ethernet header and must not be
> > > >     required to carry one (Willem de Bruijn).
> > > > ---
> > > >  net/sched/act_mpls.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> > > > index 4ea8b2e08c3a..35a6d27f76a4 100644
> > > > --- a/net/sched/act_mpls.c
> > > > +++ b/net/sched/act_mpls.c
> > > > @@ -79,6 +79,9 @@
> > > >                 if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
> > > >                                  skb->dev && skb->dev->type == ARPHRD_ETHER))
> > > >                         goto drop;
> > > > +               if (p->tcfm_proto == htons(ETH_P_TEB) &&
> > > > +                   !pskb_may_pull(skb, ETH_HLEN))
> > > > +                       goto drop;
> > > >                 break;
> > > >         case TCA_MPLS_ACT_PUSH:
> > > >                 new_lse = tcf_mpls_get_lse(NULL, p, !eth_p_mpls(skb_protocol(skb, true)));
> > > > --
> > > > 2.43.0

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Eric Dumazet]

Le lun. 26 oct. 2026 à 14:52, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> On Mon, Oct 26, 2026 at 9:10 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > Le lun. 26 oct. 2026 à 11:32, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> > >
> > > On Mon, Oct 26, 2026 at 5:42 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > >
> > > > On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > > >
> > > > > The issue:
> > > > > When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> > > > > the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> > > > > on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> > > > > following eth_type_trans() can then BUG in __skb_pull().
> > > > >
> > > > > The Fix:
> > > > > After the pop, pull the inner Ethernet header into the linear area only
> > > > > when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> > > > > fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> > > > > header and must not be required to carry one.  The pull is a no-op when
> > > > > the header is already linear.
> > > > >
> > > > > Conditions to recreate the bug:
> > > > > - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
> > > > >   (chained), or on an L3 device where mac_len == 0.
> > > > > - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
> > > > >   eth / mpls / inner-eth frame.
> > > > > - The inner header is in page frags; a following eth_type_trans() can BUG
> > > > >   in __skb_pull().
> > > > >
> > > > > Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> > > > > Reported-by: Vega <vega@nebusec.ai>
> > > > > Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> > > > > Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> > > > > Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> > > > > Cc: stable@vger.kernel.org
> > > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > > > Suggested-by: Eric Dumazet <edumazet@kernel.org>
> > > > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > > > >
> > > >
> > > > For the series
> > > >
> > > > pw-bot: cr
> > > >
> > > > Eric, you are reading this? See the discussion with Willem.
> > > >
> > >
> > > And to be clear, I am suggesting we fix both the producers and consumers.
> >
> > I don't think we need to touch loopback / eth_type_trans() directly
> > (that's the per-driver cost I pushed back on for tun). There is a single
> > natural choke point: the egress fall-through in sch_handle_egress(),
> > right before the skb goes back to the driver's ndo_start_xmit(). A
> > device advertises how much link-layer header it needs via
> > dev->min_header_len, and that is exactly the amount eth_type_trans()
> > will pull. One check there covers act_vlan, act_mpls with any inner
> > protocol, act_ife, pedit, and BPF, now and in the future:
> >
>
> You are resolving part of it with this choke point but: the primary
> concern is that ebpf can make changes that will still cause crashes at
> the consumer.
> Point is not whether you guard against bpf_skb_adjust_room() but more
> the boundary coverage i.e not correctness of the check. You are
> suggesting to (iiuc) to fix the other stuff inside ebpf? Here are some
> sample holes (there are probably others):
>
> 1) bpf_skb_adjust_room() then bpf_redirect() - note: bpf_redirect()
> returns TC_ACT_REDIRECT; sch_handle_egress() returns at the redirect
> case before the check
> 2) bpf_skb_adjust_room() then bpf_clone_redirect(); The clone is sent
> immediately inside the helper and escapes; the original skb may then
> return TC_ACT_OK and be checked
> 3) you are handlin egress only - ingress still vulnerable
>
> and xdp totally bypasses sch_handle_egress() [think
> bpf_xdp_adjust_head() then XDP_PASS or XDP redirect to veth
>
> I havent looked closely at the OVS ones you said to drop but hand wave
> review seems they can be bypassed (likely i am missing something), so
> we cant drop those.
>
> summary?
> The concern “producers alone cannot protect” remains valid unless you
> convince other wise ;-> Your turn...

We agree on the premise: producers alone cannot protect. Your bpf
skb_adjust_room case is the proof - no act_vlan/act_mpls change stops a
later program from shrinking mac. That is exactly why I went to a
boundary rather than to the producers. So the question is only whether
the egress fall-through is the *complete* boundary, and you are right to
push on that. I went and tested your holes instead of hand-waving.

Setup: inject a non-linear frame on lo via AF_PACKET+PACKET_VNET_HDR
with hdr_len=ETH_HLEN, so the linear area holds only the outer Ethernet
header and the inner one sits in frags (this geometry is what actually
BUGs; hdr_len=28 keeps the inner header linear and nothing crashes).
Each case in its own netns, PANIC_ON_OOPS=y:

  A  vlan pop_eth                                  (TC_ACT_OK)
  B  vlan pop_eth + mpls pop protocol ip           (TC_ACT_OK)
  C  vlan pop_eth + bpf skb_adjust_room(-1, MAC)   (TC_ACT_OK)
  D  vlan pop_eth + bpf_redirect(veth, 0)          (TC_ACT_REDIRECT)
  E  vlan pop_eth + bpf_redirect(lo, 0)            (TC_ACT_REDIRECT)

                          net          net + egress guard
  A  pop_eth              BUG          dropped, no BUG
  B  + mpls pop ip        BUG          dropped, no BUG
  C  + bpf shrink         BUG          dropped, no BUG
  D  + bpf_redirect veth  no BUG       no BUG
  E  + bpf_redirect lo    no BUG       no BUG

So the egress fall-through guard covers A/B/C, i.e. everything that
leaves tc on the TX path, for any inner protocol and including the BPF
shrink - no ETH_P_TEB guessing.

On your holes 1 and 2 (bpf_redirect / bpf_clone_redirect): I could not
make them BUG, on net or with the guard. The reason is that these go
through __bpf_redirect_common(), and after a pop_eth the mac header sits
exactly at the network header, so the existing

    if (skb->mac_header >= skb->network_header || skb->len == 0)
            kfree_skb(skb);

already drops the frame before it can reach eth_type_trans() (confirmed:
the skb never lands on the peer, rx_bytes does not move). clone_redirect
takes the same __bpf_redirect() path, so same outcome. In other words
that boundary is already guarded for this producer - just by a different
test than min_header_len.

The residual there is the case I flagged in my previous mail: a producer
that leaves mac_header strictly below network_header (so the check above
passes) yet with fewer than dev->min_header_len bytes linear. I could
not build one out of pop_eth / mpls pop (both leave mac == nh). If you
can craft that, that is the one I want to see - and the fix is the same
dev->min_header_len test added to __bpf_redirect_common(), not a
per-producer change.

Hole 3 (ingress): the plain RX path does not call eth_type_trans() a
second time - the mac header was already pulled by the real driver. The
only way an ingress action reaches eth_type_trans() again is by
redirecting to a device's TX/forward, and that goes through the same two
boundaries: __bpf_redirect_common() for bpf, and your act_mirred patch
(patch 3) for mirred. So ingress is covered by the same choke points,
not by the producers.

Hole 4 (XDP): agreed, and this one is genuinely separate. XDP mangles
the xdp_buff before an skb exists and bypasses sch_handle_egress()
entirely; bpf_xdp_adjust_head() + XDP_PASS/redirect is an XDP-core
question, not something an skb-level tc series can or should address. If
the build-skb path can hand eth_type_trans() a short head there, that is
its own fix. I will look, separately.

So my reading of the bounded set, for skb, is:
  { sch_handle_egress() fall-through, __bpf_redirect_common(),
    act_mirred } - a handful of boundaries, each a cheap
  dev->min_header_len test, not every producer and not eth_type_trans()
  itself (I still don't want a pull on the RX hot path).

Which maps to the series as:
  - drop patches 1 and 2 (the egress guard subsumes them; ETH_P_TEB
    question gone);
  - keep patch 3 (act_mirred) - redirect boundary;
  - keep patch 4 (act_ife) - fixes the egress frame, not just the crash;
  - keep patches 5 and 6 (OVS) as-is - different datapath, no arbitrary
    BPF, already reviewed.

If you still think 1 and 2 earn their place, the concrete thing that
would settle it is a redirect repro that BUGs with mac < nh - I will
happily eat my words. I can share the reproducers above if useful.

Thanks,

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Jamal Hadi Salim]

On Mon, Oct 26, 2026 at 10:16 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le lun. 26 oct. 2026 à 14:52, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> >
> > On Mon, Oct 26, 2026 at 9:10 AM Eric Dumazet <edumazet@kernel.org> wrote:
> > >
> > > Le lun. 26 oct. 2026 à 11:32, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> > > >
> > > > On Mon, Oct 26, 2026 at 5:42 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > > >
> > > > > On Sat, Oct 24, 2026 at 10:28 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > > > >
> > > > > > The issue:
> > > > > > When the configured inner protocol is ETH_P_TEB, act_mpls POP can leave
> > > > > > the inner Ethernet header in a page fragment after skb_mpls_pop(), either
> > > > > > on an L3 device (mac_len == 0) or chained after act_vlan POP_ETH.  A
> > > > > > following eth_type_trans() can then BUG in __skb_pull().
> > > > > >
> > > > > > The Fix:
> > > > > > After the pop, pull the inner Ethernet header into the linear area only
> > > > > > when p->tcfm_proto is ETH_P_TEB; drop the packet only when that pull
> > > > > > fails.  Other inner protocols (IP, MPLS, ...) have no inner Ethernet
> > > > > > header and must not be required to carry one.  The pull is a no-op when
> > > > > > the header is already linear.
> > > > > >
> > > > > > Conditions to recreate the bug:
> > > > > > - egress tc filter on lo: action vlan pop_eth action mpls pop protocol teb
> > > > > >   (chained), or on an L3 device where mac_len == 0.
> > > > > > - AF_PACKET SOCK_RAW with PACKET_VNET_HDR, hdr_len=28, and a non-linear
> > > > > >   eth / mpls / inner-eth frame.
> > > > > > - The inner header is in page frags; a following eth_type_trans() can BUG
> > > > > >   in __skb_pull().
> > > > > >
> > > > > > Fixes: 2a2ea50870ba ("net: sched: add mpls manipulation actions to TC")
> > > > > > Reported-by: Vega <vega@nebusec.ai>
> > > > > > Link: https://lore.kernel.org/r/40fe2d7dfce13411bfbe9271c0f7c54fb7e88e3b.1782548651.git.bronzed_45_vested@icloud.com/
> > > > > > Link: https://lore.kernel.org/netdev/CAL4WiiqSMdYQYeeW0Fk_GK6ekR4xiS4zPrGKrbB9wZknpqf1-w@mail.gmail.com/
> > > > > > Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.26d1ddb3bd2ae@gmail.com/
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > > > > Suggested-by: Eric Dumazet <edumazet@kernel.org>
> > > > > > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > > > > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > > > > >
> > > > >
> > > > > For the series
> > > > >
> > > > > pw-bot: cr
> > > > >
> > > > > Eric, you are reading this? See the discussion with Willem.
> > > > >
> > > >
> > > > And to be clear, I am suggesting we fix both the producers and consumers.
> > >
> > > I don't think we need to touch loopback / eth_type_trans() directly
> > > (that's the per-driver cost I pushed back on for tun). There is a single
> > > natural choke point: the egress fall-through in sch_handle_egress(),
> > > right before the skb goes back to the driver's ndo_start_xmit(). A
> > > device advertises how much link-layer header it needs via
> > > dev->min_header_len, and that is exactly the amount eth_type_trans()
> > > will pull. One check there covers act_vlan, act_mpls with any inner
> > > protocol, act_ife, pedit, and BPF, now and in the future:
> > >
> >
> > You are resolving part of it with this choke point but: the primary
> > concern is that ebpf can make changes that will still cause crashes at
> > the consumer.
> > Point is not whether you guard against bpf_skb_adjust_room() but more
> > the boundary coverage i.e not correctness of the check. You are
> > suggesting to (iiuc) to fix the other stuff inside ebpf? Here are some
> > sample holes (there are probably others):
> >
> > 1) bpf_skb_adjust_room() then bpf_redirect() - note: bpf_redirect()
> > returns TC_ACT_REDIRECT; sch_handle_egress() returns at the redirect
> > case before the check
> > 2) bpf_skb_adjust_room() then bpf_clone_redirect(); The clone is sent
> > immediately inside the helper and escapes; the original skb may then
> > return TC_ACT_OK and be checked
> > 3) you are handlin egress only - ingress still vulnerable
> >
> > and xdp totally bypasses sch_handle_egress() [think
> > bpf_xdp_adjust_head() then XDP_PASS or XDP redirect to veth
> >
> > I havent looked closely at the OVS ones you said to drop but hand wave
> > review seems they can be bypassed (likely i am missing something), so
> > we cant drop those.
> >
> > summary?
> > The concern “producers alone cannot protect” remains valid unless you
> > convince other wise ;-> Your turn...
>

> We agree on the premise: producers alone cannot protect. Your bpf
> skb_adjust_room case is the proof - no act_vlan/act_mpls change stops a
> later program from shrinking mac. That is exactly why I went to a
> boundary rather than to the producers. So the question is only whether
> the egress fall-through is the *complete* boundary, and you are right to
> push on that. I went and tested your holes instead of hand-waving.
>
> Setup: inject a non-linear frame on lo via AF_PACKET+PACKET_VNET_HDR
> with hdr_len=ETH_HLEN, so the linear area holds only the outer Ethernet
> header and the inner one sits in frags (this geometry is what actually
> BUGs; hdr_len=28 keeps the inner header linear and nothing crashes).
> Each case in its own netns, PANIC_ON_OOPS=y:
>
>   A  vlan pop_eth                                  (TC_ACT_OK)
>   B  vlan pop_eth + mpls pop protocol ip           (TC_ACT_OK)
>   C  vlan pop_eth + bpf skb_adjust_room(-1, MAC)   (TC_ACT_OK)
>   D  vlan pop_eth + bpf_redirect(veth, 0)          (TC_ACT_REDIRECT)
>   E  vlan pop_eth + bpf_redirect(lo, 0)            (TC_ACT_REDIRECT)
>
>                           net          net + egress guard
>   A  pop_eth              BUG          dropped, no BUG
>   B  + mpls pop ip        BUG          dropped, no BUG
>   C  + bpf shrink         BUG          dropped, no BUG
>   D  + bpf_redirect veth  no BUG       no BUG
>   E  + bpf_redirect lo    no BUG       no BUG
>
> So the egress fall-through guard covers A/B/C, i.e. everything that
> leaves tc on the TX path, for any inner protocol and including the BPF
> shrink - no ETH_P_TEB guessing.
>
> On your holes 1 and 2 (bpf_redirect / bpf_clone_redirect): I could not
> make them BUG, on net or with the guard. The reason is that these go
> through __bpf_redirect_common(), and after a pop_eth the mac header sits
> exactly at the network header, so the existing
>

I may have miscommunicated or misunderstanding you now but whay you
are describing is not the claim I was making for holes 1 and 2. The
test should be:
     action: vlan pop_eth
     action ebpf which does:
       {bpf_skb_adjust_room(skb, -1, BPF_ADJ_ROOM_MAC, 0);
        bpf_redirect(lo/veth, 0)        /* or bpf_clone_redirect() */}

From what you are describing above you did not test this sample

>     if (skb->mac_header >= skb->network_header || skb->len == 0)
>             kfree_skb(skb);
>
> already drops the frame before it can reach eth_type_trans() (confirmed:
> the skb never lands on the peer, rx_bytes does not move). clone_redirect
> takes the same __bpf_redirect() path, so same outcome. In other words
> that boundary is already guarded for this producer - just by a different
> test than min_header_len.
>
> The residual there is the case I flagged in my previous mail: a producer
> that leaves mac_header strictly below network_header (so the check above
> passes) yet with fewer than dev->min_header_len bytes linear. I could
> not build one out of pop_eth / mpls pop (both leave mac == nh). If you
> can craft that, that is the one I want to see - and the fix is the same
> dev->min_header_len test added to __bpf_redirect_common(), not a
> per-producer change.
>
> Hole 3 (ingress): the plain RX path does not call eth_type_trans() a
> second time - the mac header was already pulled by the real driver. The
> only way an ingress action reaches eth_type_trans() again is by
> redirecting to a device's TX/forward, and that goes through the same two
> boundaries: __bpf_redirect_common() for bpf, and your act_mirred patch
> (patch 3) for mirred. So ingress is covered by the same choke points,
> not by the producers.
>

There are other ways ingress action reaches eth_type_trans(); probably
a good description for this is “pop_eth + normal ingress is safe” and
not “ingress cannot reach eth_type_trans() except through redirect.”
ife action,  __dev_forward_skb2() , ovs internal_dev_recv() call it
directly (Although i admit i dont have reproducers yet for these)
IOW, plugging redirect doesnt cover all (and who knows what other
codepaths could get through in the future).

> Hole 4 (XDP): agreed, and this one is genuinely separate. XDP mangles
> the xdp_buff before an skb exists and bypasses sch_handle_egress()
> entirely; bpf_xdp_adjust_head() + XDP_PASS/redirect is an XDP-core
> question, not something an skb-level tc series can or should address. If
> the build-skb path can hand eth_type_trans() a short head there, that is
> its own fix. I will look, separately.
>

I dont see it as a separate fix tbh - it is part of the same
cause/effec bugs - i.e the same __skb_pull() BUG occurs

My position still hasnt changed at this point - and going even a bit
further: fix the consumers in "net" and this whole patch series goes
as net-next "strengthening" patch set independently. If you fix the
consumers the bug cant happen. I understand your position on
performance degradation and i empathize with it.... i am curious to
see what clever thing you come up with (you always do ;->).

cheers,
jamal


>   { sch_handle_egress() fall-through, __bpf_redirect_common(),
>     act_mirred } - a handful of boundaries, each a cheap
>   dev->min_header_len test, not every producer and not eth_type_trans()
>   itself (I still don't want a pull on the RX hot path).
>
> Which maps to the series as:
>   - drop patches 1 and 2 (the egress guard subsumes them; ETH_P_TEB
>     question gone);
>   - keep patch 3 (act_mirred) - redirect boundary;
>   - keep patch 4 (act_ife) - fixes the egress frame, not just the crash;
>   - keep patches 5 and 6 (OVS) as-is - different datapath, no arbitrary
>     BPF, already reviewed.
>
> If you still think 1 and 2 earn their place, the concrete thing that
> would settle it is a redirect repro that BUGs with mac < nh - I will
> happily eat my words. I can share the reproducers above if useful.
>
> Thanks,

Re: [PATCH net v3 2/6] net/sched: act_mpls: pull the inner Ethernet header after POP

[Eric Dumazet]

Le lun. 26 oct. 2026 à 17:02, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> My position still hasnt changed at this point - and going even a bit
> further: fix the consumers in "net" and this whole patch series goes
> as net-next "strengthening" patch set independently. If you fix the
> consumers the bug cant happen. I understand your position on
> performance degradation and i empathize with it.... i am curious to
> see what clever thing you come up with (you always do ;->).

OK, let's see what I can do.
I really want to inline eth_type_trans() in the future.