All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
@ 2026-10-27  8:25 Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:25 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
NIC drivers always provide this, and they should keep using the fast
version, without an extra pskb_may_pull().

Devices looping skbs from their transmit path back to their receive
path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
the BUG() in __skb_pull().

Instead of fixing every producer, or adding a check to all
eth_type_trans() users, fix the few consumers exposed to this:

- Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
  header first and returns a drop reason on failure.
- Patch 2 uses it in loopback_xmit().
- Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
  ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
- Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
  (TEB) and NSH pop cases at once (only the MPLS case was tested).
- Patch 5 (from Jamal) fixes act_ife decode on egress, which was
  consuming the inner Ethernet header. This is no longer a crash after
  patch 2, but egress decode never delivered a correct frame.

Other eth_type_trans() users already pull the header or never see
such skbs:
- vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
  before vrf_local_xmit().
- tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
- XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
  ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
  only trims frags.

Tested under virtme-ng with AF_PACKET reproducers. Without the series,
all four cases crash in eth_type_trans():
- lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
- lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
- lo egress "action vlan pop_eth continue", then a BPF program doing
  bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
- OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
  eth/mpls/eth/4096 bytes frame
With the series, no crash. An ICMP echo inside an IFE frame decoded
on lo egress is now received and answered. Normal traffic on lo,
veth (including 8000 byte pings) and vrf is unchanged.

Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/

Eric Dumazet (4):
  net: add eth_type_trans_careful()
  loopback: use eth_type_trans_careful()
  net: use eth_type_trans_careful() in __dev_forward_skb2()
  openvswitch: use eth_type_trans_careful() in internal_dev_recv()

Jamal Hadi Salim (1):
  net/sched: act_ife: do not consume the inner header on egress

 drivers/net/loopback.c               |  8 +++++++-
 include/linux/etherdevice.h          | 28 ++++++++++++++++++++++++++++
 net/core/dev.c                       | 18 +++++++++++++-----
 net/openvswitch/vport-internal_dev.c |  8 +++++++-
 net/sched/act_ife.c                  |  4 ++++
 5 files changed, 59 insertions(+), 7 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH net 1/5] net: add eth_type_trans_careful()
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
@ 2026-10-27  8:26 ` Eric Dumazet
  2026-10-27 13:51   ` Aaron Conole
  2026-10-27 15:02   ` Willem de Bruijn
  2026-10-27  8:26 ` [PATCH net 2/5] loopback: use eth_type_trans_careful() Eric Dumazet
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
NIC drivers always provide this, and they should keep using the fast
version.

Devices looping skbs from their transmit path back to their receive
path get skbs built by the stack instead. Their link-layer header
might have been popped by tc actions (act_vlan pop_eth, act_ife decode)
or OVS actions (pop_eth + pop_mpls / pop_nsh), leaving fewer than
ETH_HLEN bytes in the linear part. eth_type_trans() then hits
the BUG() in __skb_pull().

Add eth_type_trans_careful(), which first pulls the Ethernet header,
and returns a drop reason on failure, so that the few devices exposed
to this can drop such packets.

Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 include/linux/etherdevice.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index df8f88f63a7063fbd1df5248d2fc02c859a7bc74..1f833ebd71ed917d138ec1e64c29689ae7dd0dd5 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -38,6 +38,34 @@ int fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr);
 
 u32 eth_get_headlen(const struct net_device *dev, const void *data, u32 len);
 __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
+
+/**
+ * eth_type_trans_careful - eth_type_trans() for skbs coming from the stack
+ * @skb: buffer, possibly with less than ETH_HLEN bytes in its linear area
+ * @dev: receiving network device
+ *
+ * eth_type_trans() expects ETH_HLEN bytes in skb->head, which NIC drivers
+ * provide. Devices looping skbs from their transmit path back to the
+ * receive path (loopback, OVS internal ports) get skbs built by the stack,
+ * whose link-layer header might have been popped by tc or OVS actions.
+ *
+ * Sets skb->protocol on success.
+ *
+ * Return: SKB_NOT_DROPPED_YET on success, or a drop reason. In the latter
+ * case, the caller must free @skb.
+ */
+static inline enum skb_drop_reason
+eth_type_trans_careful(struct sk_buff *skb, struct net_device *dev)
+{
+	enum skb_drop_reason reason = pskb_may_pull_reason(skb, ETH_HLEN);
+
+	if (unlikely(reason))
+		return reason;
+
+	skb->protocol = eth_type_trans(skb, dev);
+	return SKB_NOT_DROPPED_YET;
+}
+
 extern const struct header_ops eth_header_ops;
 
 int eth_header(struct sk_buff *skb, struct net_device *dev, unsigned short type,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net 2/5] loopback: use eth_type_trans_careful()
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
@ 2026-10-27  8:26 ` Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 3/5] net: use eth_type_trans_careful() in __dev_forward_skb2() Eric Dumazet
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei, Vega

tc egress actions can leave an skb with fewer than ETH_HLEN bytes in
its linear part. loopback_xmit() then crashes in eth_type_trans():

  tc qdisc add dev lo clsact
  tc filter add dev lo egress matchall action vlan pop_eth
  # AF_PACKET SOCK_RAW: eth / eth / 8192 bytes of payload

  kernel BUG at include/linux/skbuff.h:2839!
  RIP: 0010:eth_type_trans (include/linux/etherdevice.h:635 net/ethernet/eth.c:163)
  Call Trace:
   loopback_xmit (drivers/net/loopback.c:87)
   dev_hard_start_xmit (include/linux/netdevice.h:5455 include/linux/netdevice.h:5464 net/core/dev.c:4057 net/core/dev.c:4073)
   __dev_queue_xmit (net/core/dev.c:5066)
   packet_sendmsg (net/packet/af_packet.c:3157 net/packet/af_packet.c:3189)

The same happens with "action ife decode" on lo egress.

Use eth_type_trans_careful(), and drop the packet if the Ethernet
header can not be pulled.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Vega <vega@nebusec.ai>
Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 drivers/net/loopback.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 55ecd5ebac0f21b61b64373c429a6f9ba5d15c8e..8a6cb34e6b5f02c88dfb0c6bf112068c4f51fec1 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -70,6 +70,7 @@ EXPORT_SYMBOL(blackhole_netdev);
 static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 				 struct net_device *dev)
 {
+	enum skb_drop_reason reason;
 	int len;
 
 	skb_tx_timestamp(skb);
@@ -84,7 +85,12 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
 	 */
 	skb_dst_force(skb);
 
-	skb->protocol = eth_type_trans(skb, dev);
+	reason = eth_type_trans_careful(skb, dev);
+	if (unlikely(reason)) {
+		dev_core_stats_tx_dropped_inc(dev);
+		kfree_skb_reason(skb, reason);
+		return NETDEV_TX_OK;
+	}
 
 	len = skb->len;
 	if (likely(__netif_rx(skb) == NET_RX_SUCCESS))
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net 3/5] net: use eth_type_trans_careful() in __dev_forward_skb2()
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 2/5] loopback: use eth_type_trans_careful() Eric Dumazet
@ 2026-10-27  8:26 ` Eric Dumazet
  2026-10-27  8:26 ` [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv() Eric Dumazet
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

A tc egress BPF program can push a tiny link-layer header with
bpf_skb_change_head() on an skb whose head is almost empty, then
redirect it to an Ethernet device ingress. __bpf_redirect_common()
only checks mac_header < network_header, and __dev_forward_skb2()
crashes in eth_type_trans():

  tc filter add dev lo egress pref 1 matchall action vlan pop_eth continue
  tc filter add dev lo egress pref 2 bpf da obj prog.o
  # prog: bpf_skb_change_head(skb, 1, 0);
  #       return bpf_redirect(veth1_ifindex, BPF_F_INGRESS);
  # AF_PACKET SOCK_RAW on lo: eth / eth / 8192 bytes of payload

  kernel BUG at include/linux/skbuff.h:2839!
  RIP: 0010:eth_type_trans (include/linux/etherdevice.h:635 net/ethernet/eth.c:163)
  Call Trace:
   __dev_forward_skb2 (net/core/dev.c:2507)
   dev_forward_skb_nomtu (net/core/dev.c:2546)
   __dev_queue_xmit (net/core/dev.c:4738 net/core/dev.c:4998)
   packet_sendmsg (net/packet/af_packet.c:3157 net/packet/af_packet.c:3189)

Use eth_type_trans_careful(), and drop the packet if the Ethernet
header can not be pulled. This also covers other dev_forward_skb()
users (veth, macvlan, ipvlan, netdevsim, l2tp_eth).

Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/core/dev.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a09e7869118059e92b53e00ed873dfc7485ffc3..ac784310a69c1d9ceecf68909334196b555871c5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2501,14 +2501,22 @@ EXPORT_SYMBOL_GPL(is_skb_forwardable);
 static int __dev_forward_skb2(struct net_device *dev, struct sk_buff *skb,
 			      bool check_mtu)
 {
-	int ret = ____dev_forward_skb(dev, skb, check_mtu);
+	enum skb_drop_reason reason;
+	int ret;
+
+	ret = ____dev_forward_skb(dev, skb, check_mtu);
+	if (unlikely(ret))
+		return ret;
 
-	if (likely(!ret)) {
-		skb->protocol = eth_type_trans(skb, dev);
-		skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
+	reason = eth_type_trans_careful(skb, dev);
+	if (unlikely(reason)) {
+		dev_core_stats_rx_dropped_inc(dev);
+		kfree_skb_reason(skb, reason);
+		return NET_RX_DROP;
 	}
+	skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
 
-	return ret;
+	return 0;
 }
 
 int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv()
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
                   ` (2 preceding siblings ...)
  2026-10-27  8:26 ` [PATCH net 3/5] net: use eth_type_trans_careful() in __dev_forward_skb2() Eric Dumazet
@ 2026-10-27  8:26 ` Eric Dumazet
  2026-10-27 13:50   ` Aaron Conole
  2026-10-27  8:26 ` [PATCH net 5/5] net/sched: act_ife: do not consume the inner header on egress Eric Dumazet
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

OVS actions like pop_eth followed by pop_mpls(ETH_P_TEB) or pop_nsh
can expose an inner Ethernet header which is not in the linear part
of the skb. Output to an internal port then crashes in eth_type_trans():

  flow: in_port(0),eth(),eth_type(0x8847),mpls(lse=0x00010140)
  actions: pop_eth,pop_mpls(0x6558),0
  # AF_PACKET SOCK_RAW on dp0: eth / mpls / eth / 4096 bytes of payload

  kernel BUG at include/linux/skbuff.h:2839!
  RIP: 0010:eth_type_trans (include/linux/etherdevice.h:635 net/ethernet/eth.c:163)
  Call Trace:
   internal_dev_recv (net/openvswitch/vport-internal_dev.c:202) openvswitch
   do_execute_actions (net/openvswitch/actions.c:1296) openvswitch
   ovs_execute_actions (net/openvswitch/actions.c:1606) openvswitch
   ovs_dp_process_packet (net/openvswitch/datapath.c:310) openvswitch
   ovs_vport_receive (net/openvswitch/vport.c:448) openvswitch
   internal_dev_xmit (net/openvswitch/vport-internal_dev.c:42) openvswitch
   dev_hard_start_xmit (include/linux/netdevice.h:5455 include/linux/netdevice.h:5464 net/core/dev.c:4057 net/core/dev.c:4073)
   __dev_queue_xmit (net/core/dev.c:5066)
   packet_sendmsg (net/packet/af_packet.c:3157 net/packet/af_packet.c:3189)

Use eth_type_trans_careful(), and drop the packet if the Ethernet
header can not be pulled. This covers all OVS actions at the consumer,
instead of fixing each producer.

Fixes: b2d0f5d5dc53 ("openvswitch: enable NSH support")
Fixes: f66b53fdbb22 ("openvswitch: New MPLS actions for layer 2 tunnelling")
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 net/openvswitch/vport-internal_dev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 77a16d5ebd54343c80a2def261aadcb8ff0d1274..8f035087f0eec80eddf6b493cf5ba7915a3e01c6 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -188,6 +188,7 @@ static void internal_dev_destroy(struct vport *vport)
 static int internal_dev_recv(struct sk_buff *skb)
 {
 	struct net_device *netdev = skb->dev;
+	enum skb_drop_reason reason;
 
 	if (unlikely(!(netdev->flags & IFF_UP))) {
 		kfree_skb(skb);
@@ -199,7 +200,12 @@ static int internal_dev_recv(struct sk_buff *skb)
 	nf_reset_ct(skb);
 
 	skb->pkt_type = PACKET_HOST;
-	skb->protocol = eth_type_trans(skb, netdev);
+	reason = eth_type_trans_careful(skb, netdev);
+	if (unlikely(reason)) {
+		dev_core_stats_rx_dropped_inc(netdev);
+		kfree_skb_reason(skb, reason);
+		return NETDEV_TX_OK;
+	}
 	skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
 	dev_sw_netstats_rx_add(netdev, skb->len);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net 5/5] net/sched: act_ife: do not consume the inner header on egress
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
                   ` (3 preceding siblings ...)
  2026-10-27  8:26 ` [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv() Eric Dumazet
@ 2026-10-27  8:26 ` Eric Dumazet
  2026-10-27  9:17 ` [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Jamal Hadi Salim
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  8:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Jamal Hadi Salim,
	Andrew Lunn, Aaron Conole, Eelco Chaudron, Ilya Maximets,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei, stable, Eric Dumazet

From: Jamal Hadi Salim <jhs@mojatatu.com>

The issue:
tcf_ife_decode() unconditionally calls eth_type_trans(), which pulls
ETH_HLEN.  On egress this consumes the inner Ethernet header, and the
device receives an skb whose data points to the L3 header:

- loopback_xmit() parses L3 data as Ethernet: the "ethertype" comes
  from the inner IPv4 source address, and the decapsulated packet is
  dropped or misparsed.
- a real NIC transmits the frame without its Ethernet header.

(The __skb_pull() BUG this used to trigger in loopback_xmit() when the
remaining linear area was short is handled by "loopback: use
eth_type_trans_careful()".)

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.

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 + a valid IPv4 ICMP echo request to a local address;
- before this patch, no echo is received; after it, the echo is
  delivered and answered.

Fixes: ef6980b6becb ("introduce IFE action")
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
[edumazet: reworded changelog, the crash is now handled in loopback]
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
 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 2afd68983ece40d8cae6c96af86e2ea65be476bc..3946a9589bb8fcd296e6f1f9234c00238df4d954 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.53.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
                   ` (4 preceding siblings ...)
  2026-10-27  8:26 ` [PATCH net 5/5] net/sched: act_ife: do not consume the inner header on egress Eric Dumazet
@ 2026-10-27  9:17 ` Jamal Hadi Salim
  2026-10-27  9:51   ` Eric Dumazet
  2026-10-27 14:40 ` Willem de Bruijn
  2026-10-27 21:25 ` Jakub Kicinski
  7 siblings, 1 reply; 21+ messages in thread
From: Jamal Hadi Salim @ 2026-10-27  9:17 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron,
	Ilya Maximets, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

On Tue, Oct 27, 2026 at 4:26 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> NIC drivers always provide this, and they should keep using the fast
> version, without an extra pskb_may_pull().
>
> Devices looping skbs from their transmit path back to their receive
> path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> the BUG() in __skb_pull().
>
> Instead of fixing every producer, or adding a check to all
> eth_type_trans() users, fix the few consumers exposed to this:
>
> - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
>   header first and returns a drop reason on failure.
> - Patch 2 uses it in loopback_xmit().
> - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
>   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
>   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
>   consuming the inner Ethernet header. This is no longer a crash after
>   patch 2, but egress decode never delivered a correct frame.
>
> Other eth_type_trans() users already pull the header or never see
> such skbs:
> - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
>   before vrf_local_xmit().
> - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
>   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
>   only trims frags.
>
> Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> all four cases crash in eth_type_trans():
> - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> - lo egress "action vlan pop_eth continue", then a BPF program doing
>   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
>   eth/mpls/eth/4096 bytes frame
> With the series, no crash. An ICMP echo inside an IFE frame decoded
> on lo egress is now received and answered. Normal traffic on lo,
> veth (including 8000 byte pings) and vrf is unchanged.
>
> Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
>

Looks interesting ;-> I dont think you are covering xdp/bpf holes but
we can worry about that later.
I will spend time later this morning looking closely and actually
running it. Will keep you posted.

cheers,
jamal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27  9:17 ` [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Jamal Hadi Salim
@ 2026-10-27  9:51   ` Eric Dumazet
  2026-10-27 13:07     ` Jamal Hadi Salim
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27  9:51 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron,
	Ilya Maximets, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Le mar. 27 oct. 2026 à 10:17, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> On Tue, Oct 27, 2026 at 4:26 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > NIC drivers always provide this, and they should keep using the fast
> > version, without an extra pskb_may_pull().
> >
> > Devices looping skbs from their transmit path back to their receive
> > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > the BUG() in __skb_pull().
> >
> > Instead of fixing every producer, or adding a check to all
> > eth_type_trans() users, fix the few consumers exposed to this:
> >
> > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
> >   header first and returns a drop reason on failure.
> > - Patch 2 uses it in loopback_xmit().
> > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
> >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
> >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
> >   consuming the inner Ethernet header. This is no longer a crash after
> >   patch 2, but egress decode never delivered a correct frame.
> >
> > Other eth_type_trans() users already pull the header or never see
> > such skbs:
> > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
> >   before vrf_local_xmit().
> > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
> >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
> >   only trims frags.
> >
> > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> > all four cases crash in eth_type_trans():
> > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> > - lo egress "action vlan pop_eth continue", then a BPF program doing
> >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
> >   eth/mpls/eth/4096 bytes frame
> > With the series, no crash. An ICMP echo inside an IFE frame decoded
> > on lo egress is now received and answered. Normal traffic on lo,
> > veth (including 8000 byte pings) and vrf is unchanged.
> >
> > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> >
>
> Looks interesting ;-> I dont think you are covering xdp/bpf holes but
> we can worry about that later.
> I will spend time later this morning looking closely and actually
> running it. Will keep you posted.

Thanks Jamal.

I went through the holes you listed earlier:

1) bpf_skb_adjust_room() + bpf_redirect():
   An egress redirect ends in the target ndo_start_xmit():
   loopback (patch 2), veth (already pulls ETH_HLEN), OVS internal
   port (patch 4). An ingress redirect goes through
   __dev_forward_skb2() (patch 3). The repro in patch 3 uses
   bpf_skb_change_head(), the only helper I found that leaves
   1 <= mac_len < ETH_HLEN with an almost empty head, passing the
   mac_header < network_header check in __bpf_redirect_common().

2) bpf_clone_redirect(): same __bpf_redirect() path as 1).

3) ingress: after tc ingress, eth_type_trans() is only called again
   if the packet is redirected, see 1).

4) XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() refuse to
   leave less than ETH_HLEN bytes in the linear part, and multi-buffer
   tail shrinking only trims frags. skbs built from xdp buffers
   (XDP_PASS, veth XDP, generic XDP) have a full Ethernet header in
   skb->head.

Let's use something that can be plugged / unplugged easily in the future.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27  9:51   ` Eric Dumazet
@ 2026-10-27 13:07     ` Jamal Hadi Salim
  2026-10-27 14:00       ` Aaron Conole
  2026-10-27 14:33       ` Eric Dumazet
  0 siblings, 2 replies; 21+ messages in thread
From: Jamal Hadi Salim @ 2026-10-27 13:07 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron,
	Ilya Maximets, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

[-- Attachment #1: Type: text/plain, Size: 5111 bytes --]

On Tue, Oct 27, 2026 at 5:52 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le mar. 27 oct. 2026 à 10:17, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> >
> > On Tue, Oct 27, 2026 at 4:26 AM Eric Dumazet <edumazet@kernel.org> wrote:
> > >
> > > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > > NIC drivers always provide this, and they should keep using the fast
> > > version, without an extra pskb_may_pull().
> > >
> > > Devices looping skbs from their transmit path back to their receive
> > > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > > the BUG() in __skb_pull().
> > >
> > > Instead of fixing every producer, or adding a check to all
> > > eth_type_trans() users, fix the few consumers exposed to this:
> > >
> > > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
> > >   header first and returns a drop reason on failure.
> > > - Patch 2 uses it in loopback_xmit().
> > > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
> > >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> > > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
> > >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> > > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
> > >   consuming the inner Ethernet header. This is no longer a crash after
> > >   patch 2, but egress decode never delivered a correct frame.
> > >
> > > Other eth_type_trans() users already pull the header or never see
> > > such skbs:
> > > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
> > >   before vrf_local_xmit().
> > > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> > > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
> > >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
> > >   only trims frags.
> > >
> > > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> > > all four cases crash in eth_type_trans():
> > > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> > > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> > > - lo egress "action vlan pop_eth continue", then a BPF program doing
> > >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> > > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
> > >   eth/mpls/eth/4096 bytes frame
> > > With the series, no crash. An ICMP echo inside an IFE frame decoded
> > > on lo egress is now received and answered. Normal traffic on lo,
> > > veth (including 8000 byte pings) and vrf is unchanged.
> > >
> > > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> > > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> > >
> >
> > Looks interesting ;-> I dont think you are covering xdp/bpf holes but
> > we can worry about that later.
> > I will spend time later this morning looking closely and actually
> > running it. Will keep you posted.
>
> Thanks Jamal.
>
> I went through the holes you listed earlier:
>
> 1) bpf_skb_adjust_room() + bpf_redirect():
>    An egress redirect ends in the target ndo_start_xmit():
>    loopback (patch 2), veth (already pulls ETH_HLEN), OVS internal
>    port (patch 4). An ingress redirect goes through
>    __dev_forward_skb2() (patch 3). The repro in patch 3 uses
>    bpf_skb_change_head(), the only helper I found that leaves
>    1 <= mac_len < ETH_HLEN with an almost empty head, passing the
>    mac_header < network_header check in __bpf_redirect_common().
>
> 2) bpf_clone_redirect(): same __bpf_redirect() path as 1).
>
> 3) ingress: after tc ingress, eth_type_trans() is only called again
>    if the packet is redirected, see 1).
>
> 4) XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() refuse to
>    leave less than ETH_HLEN bytes in the linear part, and multi-buffer
>    tail shrinking only trims frags. skbs built from xdp buffers
>    (XDP_PASS, veth XDP, generic XDP) have a full Ethernet header in
>    skb->head.
>
> Let's use something that can be plugged / unplugged easily in the future.

I reviewed and tested all existing issue, they are fixed - kudos ;->
But, but.... you missed one;-> see attached patch.

Additional questions:
1) Would be interesting to see what performance degradation happened here.
2) I think it is reasonable i send my original patches 1-3 to net-next
when it opens as "hardening" patches
3) Is it correct to put multiple Fixes: in net patches (you have i
think 4 in one of them)? Sashiko complained once that a patch I sent
made stable backporting hard. Either you pick one or split into
multiple patches. I realize picking a "good" fix tag is very hard in
this case.

cheers,
jamal

[-- Attachment #2: bpf_redirect_neigh.patch --]
[-- Type: application/octet-stream, Size: 2072 bytes --]

From 624ff9ce40f57f98176c23bac4dedd29ab572745 Mon Sep 17 00:00:00 2001
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Tue, 27 Oct 2026 08:07:59 -0400
Subject: [PATCH net] bpf: Fix skb_pull() BUG in __bpf_redirect_neigh()
To: Eric Dumazet <edumazet@kernel.org>
Cc: netdev@vger.kernel.org,
    bpf@vger.kernel.org,
    stable@vger.kernel.org

__bpf_redirect_neigh() calls skb_pull(skb, sizeof(*ethh)) after
checking only that mac_header < network_header.  It does not check
that the MAC header is long enough or that it is linear.

A SCHED_ACT BPF program can do:

    bpf_skb_change_head(skb, 1, 0);
    bpf_redirect_neigh(lo, NULL, 0, 0);

after vlan pop_eth.  The skb then has mac_len == 1 and only one byte
of linear data; skb_pull(ETH_HLEN) trips the skb->len < skb->data_len
BUG in __skb_pull():

    kernel BUG at include/linux/skbuff.h:2839
    RIP: skb_pull+0xc5/0x100
    Call Trace: skb_do_redirect -> ...

Drop skbs whose MAC header is shorter than ETH_HLEN, and use
pskb_may_pull() to ensure the Ethernet header is linear before reading
or pulling it.  Re-read ethh after pskb_may_pull(), which may
reallocate skb->head.

Fixes: b4ab31414970 ("bpf: Add redirect_neigh helper as redirect drop-in")
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/core/filter.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 70dc621672f2..4d344ec09736 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2432,10 +2432,14 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
 static int __bpf_redirect_neigh(struct sk_buff *skb, struct net_device *dev,
 				struct bpf_nh_params *nh)
 {
-	struct ethhdr *ethh = eth_hdr(skb);
+	struct ethhdr *ethh;
 
 	if (unlikely(skb->mac_header >= skb->network_header))
 		goto out;
+	if (unlikely(skb->mac_len < ETH_HLEN ||
+		     !pskb_may_pull(skb, ETH_HLEN)))
+		goto out;
+	ethh = eth_hdr(skb);
 	bpf_push_mac_rcsum(skb);
 	if (is_multicast_ether_addr(ethh->h_dest))
 		goto out;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv()
  2026-10-27  8:26 ` [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv() Eric Dumazet
@ 2026-10-27 13:50   ` Aaron Conole
  0 siblings, 0 replies; 21+ messages in thread
From: Aaron Conole @ 2026-10-27 13:50 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Eelco Chaudron, Ilya Maximets,
	Jamal Hadi Salim, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Eric Dumazet <edumazet@kernel.org> writes:

> OVS actions like pop_eth followed by pop_mpls(ETH_P_TEB) or pop_nsh
> can expose an inner Ethernet header which is not in the linear part
> of the skb. Output to an internal port then crashes in eth_type_trans():
>
>   flow: in_port(0),eth(),eth_type(0x8847),mpls(lse=0x00010140)
>   actions: pop_eth,pop_mpls(0x6558),0
>   # AF_PACKET SOCK_RAW on dp0: eth / mpls / eth / 4096 bytes of payload
>
>   kernel BUG at include/linux/skbuff.h:2839!
>   RIP: 0010:eth_type_trans (include/linux/etherdevice.h:635 net/ethernet/eth.c:163)
>   Call Trace:
>    internal_dev_recv (net/openvswitch/vport-internal_dev.c:202) openvswitch
>    do_execute_actions (net/openvswitch/actions.c:1296) openvswitch
>    ovs_execute_actions (net/openvswitch/actions.c:1606) openvswitch
>    ovs_dp_process_packet (net/openvswitch/datapath.c:310) openvswitch
>    ovs_vport_receive (net/openvswitch/vport.c:448) openvswitch
>    internal_dev_xmit (net/openvswitch/vport-internal_dev.c:42) openvswitch
>    dev_hard_start_xmit (include/linux/netdevice.h:5455 include/linux/netdevice.h:5464 net/core/dev.c:4057 net/core/dev.c:4073)
>    __dev_queue_xmit (net/core/dev.c:5066)
>    packet_sendmsg (net/packet/af_packet.c:3157 net/packet/af_packet.c:3189)
>
> Use eth_type_trans_careful(), and drop the packet if the Ethernet
> header can not be pulled. This covers all OVS actions at the consumer,
> instead of fixing each producer.
>
> Fixes: b2d0f5d5dc53 ("openvswitch: enable NSH support")
> Fixes: f66b53fdbb22 ("openvswitch: New MPLS actions for layer 2 tunnelling")
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---

Thanks Eric, this looks good to me.  I guess Jamal's proposed changes
for OVS/TC aren't needed with this series.

Reviewed-by: Aaron Conole <aconole@redhat.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 1/5] net: add eth_type_trans_careful()
  2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
@ 2026-10-27 13:51   ` Aaron Conole
  2026-10-27 15:02   ` Willem de Bruijn
  1 sibling, 0 replies; 21+ messages in thread
From: Aaron Conole @ 2026-10-27 13:51 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Eelco Chaudron, Ilya Maximets,
	Jamal Hadi Salim, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Eric Dumazet <edumazet@kernel.org> writes:

> eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> NIC drivers always provide this, and they should keep using the fast
> version.
>
> Devices looping skbs from their transmit path back to their receive
> path get skbs built by the stack instead. Their link-layer header
> might have been popped by tc actions (act_vlan pop_eth, act_ife decode)
> or OVS actions (pop_eth + pop_mpls / pop_nsh), leaving fewer than
> ETH_HLEN bytes in the linear part. eth_type_trans() then hits
> the BUG() in __skb_pull().
>
> Add eth_type_trans_careful(), which first pulls the Ethernet header,
> and returns a drop reason on failure, so that the few devices exposed
> to this can drop such packets.
>
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---

Reviewed-by: Aaron Conole <aconole@redhat.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 13:07     ` Jamal Hadi Salim
@ 2026-10-27 14:00       ` Aaron Conole
  2026-10-27 14:33       ` Eric Dumazet
  1 sibling, 0 replies; 21+ messages in thread
From: Aaron Conole @ 2026-10-27 14:00 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Willem de Bruijn, Simon Horman, netdev, Andrew Lunn,
	Eelco Chaudron, Ilya Maximets, Jiri Pirko, dev, Willem de Bruijn,
	Ren Wei

Jamal Hadi Salim <jhs@mojatatu.com> writes:

> On Tue, Oct 27, 2026 at 5:52 AM Eric Dumazet <edumazet@kernel.org> wrote:
>>
>> Le mar. 27 oct. 2026 à 10:17, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>> >
>> > On Tue, Oct 27, 2026 at 4:26 AM Eric Dumazet <edumazet@kernel.org> wrote:
>> > >
>> > > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
>> > > NIC drivers always provide this, and they should keep using the fast
>> > > version, without an extra pskb_may_pull().
>> > >
>> > > Devices looping skbs from their transmit path back to their receive
>> > > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
>> > > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
>> > > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
>> > > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
>> > > the BUG() in __skb_pull().
>> > >
>> > > Instead of fixing every producer, or adding a check to all
>> > > eth_type_trans() users, fix the few consumers exposed to this:
>> > >
>> > > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
>> > >   header first and returns a drop reason on failure.
>> > > - Patch 2 uses it in loopback_xmit().
>> > > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
>> > >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
>> > > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
>> > >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
>> > > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
>> > >   consuming the inner Ethernet header. This is no longer a crash after
>> > >   patch 2, but egress decode never delivered a correct frame.
>> > >
>> > > Other eth_type_trans() users already pull the header or never see
>> > > such skbs:
>> > > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
>> > >   before vrf_local_xmit().
>> > > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
>> > > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
>> > >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
>> > >   only trims frags.
>> > >
>> > > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
>> > > all four cases crash in eth_type_trans():
>> > > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
>> > > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
>> > > - lo egress "action vlan pop_eth continue", then a BPF program doing
>> > >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
>> > > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
>> > >   eth/mpls/eth/4096 bytes frame
>> > > With the series, no crash. An ICMP echo inside an IFE frame decoded
>> > > on lo egress is now received and answered. Normal traffic on lo,
>> > > veth (including 8000 byte pings) and vrf is unchanged.
>> > >
>> > > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
>> > > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
>> > >
>> >
>> > Looks interesting ;-> I dont think you are covering xdp/bpf holes but
>> > we can worry about that later.
>> > I will spend time later this morning looking closely and actually
>> > running it. Will keep you posted.
>>
>> Thanks Jamal.
>>
>> I went through the holes you listed earlier:
>>
>> 1) bpf_skb_adjust_room() + bpf_redirect():
>>    An egress redirect ends in the target ndo_start_xmit():
>>    loopback (patch 2), veth (already pulls ETH_HLEN), OVS internal
>>    port (patch 4). An ingress redirect goes through
>>    __dev_forward_skb2() (patch 3). The repro in patch 3 uses
>>    bpf_skb_change_head(), the only helper I found that leaves
>>    1 <= mac_len < ETH_HLEN with an almost empty head, passing the
>>    mac_header < network_header check in __bpf_redirect_common().
>>
>> 2) bpf_clone_redirect(): same __bpf_redirect() path as 1).
>>
>> 3) ingress: after tc ingress, eth_type_trans() is only called again
>>    if the packet is redirected, see 1).
>>
>> 4) XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() refuse to
>>    leave less than ETH_HLEN bytes in the linear part, and multi-buffer
>>    tail shrinking only trims frags. skbs built from xdp buffers
>>    (XDP_PASS, veth XDP, generic XDP) have a full Ethernet header in
>>    skb->head.
>>
>> Let's use something that can be plugged / unplugged easily in the future.
>
> I reviewed and tested all existing issue, they are fixed - kudos ;->
> But, but.... you missed one;-> see attached patch.
>
> Additional questions:
> 1) Would be interesting to see what performance degradation happened here.
> 2) I think it is reasonable i send my original patches 1-3 to net-next
> when it opens as "hardening" patches
> 3) Is it correct to put multiple Fixes: in net patches (you have i
> think 4 in one of them)? Sashiko complained once that a patch I sent
> made stable backporting hard. Either you pick one or split into
> multiple patches. I realize picking a "good" fix tag is very hard in
> this case.

I don't believe multiple fixes is disallowed.  At least for OVS side, we
recently had:

0dbc2398fca3 ("openvswitch: only skb_tx_error() a packet we are about to drop")

This tags multiple fixes (dated Aug 22).  And I think 'net' has a few as
well (like 06e3f54e8b22).

Maybe Sashiko was feeling ornery, although the complaint that backports
can be harder is probably true :)

> cheers,
> jamal


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 13:07     ` Jamal Hadi Salim
  2026-10-27 14:00       ` Aaron Conole
@ 2026-10-27 14:33       ` Eric Dumazet
  1 sibling, 0 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27 14:33 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron,
	Ilya Maximets, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Le mar. 27 oct. 2026 à 14:07, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
>
> On Tue, Oct 27, 2026 at 5:52 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > Le mar. 27 oct. 2026 à 10:17, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> > >
> > > On Tue, Oct 27, 2026 at 4:26 AM Eric Dumazet <edumazet@kernel.org> wrote:
> > > >
> > > > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > > > NIC drivers always provide this, and they should keep using the fast
> > > > version, without an extra pskb_may_pull().
> > > >
> > > > Devices looping skbs from their transmit path back to their receive
> > > > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > > > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > > > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > > > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > > > the BUG() in __skb_pull().
> > > >
> > > > Instead of fixing every producer, or adding a check to all
> > > > eth_type_trans() users, fix the few consumers exposed to this:
> > > >
> > > > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
> > > >   header first and returns a drop reason on failure.
> > > > - Patch 2 uses it in loopback_xmit().
> > > > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
> > > >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> > > > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
> > > >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> > > > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
> > > >   consuming the inner Ethernet header. This is no longer a crash after
> > > >   patch 2, but egress decode never delivered a correct frame.
> > > >
> > > > Other eth_type_trans() users already pull the header or never see
> > > > such skbs:
> > > > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
> > > >   before vrf_local_xmit().
> > > > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> > > > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
> > > >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
> > > >   only trims frags.
> > > >
> > > > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> > > > all four cases crash in eth_type_trans():
> > > > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> > > > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> > > > - lo egress "action vlan pop_eth continue", then a BPF program doing
> > > >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> > > > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
> > > >   eth/mpls/eth/4096 bytes frame
> > > > With the series, no crash. An ICMP echo inside an IFE frame decoded
> > > > on lo egress is now received and answered. Normal traffic on lo,
> > > > veth (including 8000 byte pings) and vrf is unchanged.
> > > >
> > > > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> > > > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> > > >
> > >
> > > Looks interesting ;-> I dont think you are covering xdp/bpf holes but
> > > we can worry about that later.
> > > I will spend time later this morning looking closely and actually
> > > running it. Will keep you posted.
> >
> > Thanks Jamal.
> >
> > I went through the holes you listed earlier:
> >
> > 1) bpf_skb_adjust_room() + bpf_redirect():
> >    An egress redirect ends in the target ndo_start_xmit():
> >    loopback (patch 2), veth (already pulls ETH_HLEN), OVS internal
> >    port (patch 4). An ingress redirect goes through
> >    __dev_forward_skb2() (patch 3). The repro in patch 3 uses
> >    bpf_skb_change_head(), the only helper I found that leaves
> >    1 <= mac_len < ETH_HLEN with an almost empty head, passing the
> >    mac_header < network_header check in __bpf_redirect_common().
> >
> > 2) bpf_clone_redirect(): same __bpf_redirect() path as 1).
> >
> > 3) ingress: after tc ingress, eth_type_trans() is only called again
> >    if the packet is redirected, see 1).
> >
> > 4) XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() refuse to
> >    leave less than ETH_HLEN bytes in the linear part, and multi-buffer
> >    tail shrinking only trims frags. skbs built from xdp buffers
> >    (XDP_PASS, veth XDP, generic XDP) have a full Ethernet header in
> >    skb->head.
> >
> > Let's use something that can be plugged / unplugged easily in the future.
>
> I reviewed and tested all existing issue, they are fixed - kudos ;->
> But, but.... you missed one;-> see attached patch.

Yes, I mostly focused on eth_type_trans().

>
> Additional questions:
> 1) Would be interesting to see what performance degradation happened here.

NIC drivers still use the plain eth_type_trans().
Only loopback, __dev_forward_skb2() and OVS internal ports pay for one
inline skb_headlen() comparison.

> 2) I think it is reasonable i send my original patches 1-3 to net-next
> when it opens as "hardening" patches

  Yes.

> 3) Is it correct to put multiple Fixes: in net patches (you have i
> think 4 in one of them)? Sashiko complained once that a patch I sent
> made stable backporting hard. Either you pick one or split into
> multiple patches. I realize picking a "good" fix tag is very hard in
> this case.

Yes,  both are real origins. Stable can use the oldest. That's common practice.

>
> cheers,
> jamal

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
                   ` (5 preceding siblings ...)
  2026-10-27  9:17 ` [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Jamal Hadi Salim
@ 2026-10-27 14:40 ` Willem de Bruijn
  2026-10-27 15:49   ` Eric Dumazet
  2026-10-27 21:25 ` Jakub Kicinski
  7 siblings, 1 reply; 21+ messages in thread
From: Willem de Bruijn @ 2026-10-27 14:40 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Eric Dumazet wrote:
> eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> NIC drivers always provide this, and they should keep using the fast
> version, without an extra pskb_may_pull().
> 
> Devices looping skbs from their transmit path back to their receive
> path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> the BUG() in __skb_pull().
> 
> Instead of fixing every producer, or adding a check to all
> eth_type_trans() users, fix the few consumers exposed to this:

Definitely a nice approach.

You previously suggested catching most of the TC modifications with a
single check in sch_handle_(e|in)gress. Is that no longer needed?

The few mirred/redirect cases are a bane, causing so many issues. And
escape any such check, as they call netif_receive_skb, netif_rx,
dev_queue_xmit, directly.

If it's only act_mirred, bpf_redirect and OVS, maybe a single
validation helper across all three before they pass to the rx/tx
path, that for starters calls eth_type_trans_careful. It quite likely
won't be the last packet corruption they are capable of..

> 
> - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
>   header first and returns a drop reason on failure.
> - Patch 2 uses it in loopback_xmit().
> - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
>   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
>   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
>   consuming the inner Ethernet header. This is no longer a crash after
>   patch 2, but egress decode never delivered a correct frame.
> 
> Other eth_type_trans() users already pull the header or never see
> such skbs:
> - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
>   before vrf_local_xmit().
> - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
>   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
>   only trims frags.
> 
> Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> all four cases crash in eth_type_trans():
> - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> - lo egress "action vlan pop_eth continue", then a BPF program doing
>   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
>   eth/mpls/eth/4096 bytes frame
> With the series, no crash. An ICMP echo inside an IFE frame decoded
> on lo egress is now received and answered. Normal traffic on lo,
> veth (including 8000 byte pings) and vrf is unchanged.
> 
> Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> 
> Eric Dumazet (4):
>   net: add eth_type_trans_careful()
>   loopback: use eth_type_trans_careful()
>   net: use eth_type_trans_careful() in __dev_forward_skb2()
>   openvswitch: use eth_type_trans_careful() in internal_dev_recv()
> 
> Jamal Hadi Salim (1):
>   net/sched: act_ife: do not consume the inner header on egress
> 
>  drivers/net/loopback.c               |  8 +++++++-
>  include/linux/etherdevice.h          | 28 ++++++++++++++++++++++++++++
>  net/core/dev.c                       | 18 +++++++++++++-----
>  net/openvswitch/vport-internal_dev.c |  8 +++++++-
>  net/sched/act_ife.c                  |  4 ++++
>  5 files changed, 59 insertions(+), 7 deletions(-)
> 
> -- 
> 2.53.0
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 1/5] net: add eth_type_trans_careful()
  2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
  2026-10-27 13:51   ` Aaron Conole
@ 2026-10-27 15:02   ` Willem de Bruijn
  1 sibling, 0 replies; 21+ messages in thread
From: Willem de Bruijn @ 2026-10-27 15:02 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, Simon Horman, netdev, Eric Dumazet, Andrew Lunn,
	Aaron Conole, Eelco Chaudron, Ilya Maximets, Jamal Hadi Salim,
	Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Eric Dumazet wrote:
> eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> NIC drivers always provide this, and they should keep using the fast
> version.
> 
> Devices looping skbs from their transmit path back to their receive
> path get skbs built by the stack instead. Their link-layer header
> might have been popped by tc actions (act_vlan pop_eth, act_ife decode)
> or OVS actions (pop_eth + pop_mpls / pop_nsh), leaving fewer than
> ETH_HLEN bytes in the linear part. eth_type_trans() then hits
> the BUG() in __skb_pull().
> 
> Add eth_type_trans_careful(), which first pulls the Ethernet header,
> and returns a drop reason on failure, so that the few devices exposed
> to this can drop such packets.
> 
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 14:40 ` Willem de Bruijn
@ 2026-10-27 15:49   ` Eric Dumazet
  2026-10-27 19:48     ` Jamal Hadi Salim
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27 15:49 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron,
	Ilya Maximets, Jamal Hadi Salim, Jiri Pirko, dev, Ren Wei

Le mar. 27 oct. 2026 à 15:40, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> a écrit :
>
> Eric Dumazet wrote:
> > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > NIC drivers always provide this, and they should keep using the fast
> > version, without an extra pskb_may_pull().
> >
> > Devices looping skbs from their transmit path back to their receive
> > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > the BUG() in __skb_pull().
> >
> > Instead of fixing every producer, or adding a check to all
> > eth_type_trans() users, fix the few consumers exposed to this:
>
> Definitely a nice approach.

Thanks!

>
> You previously suggested catching most of the TC modifications with a
> single check in sch_handle_(e|in)gress. Is that no longer needed?

Not for these crashes. The sch_handle_egress() check would only cover
tc egress. It would miss OVS and the redirect paths you mention, and it
would add a check for every packet going through clsact. Fixing the
consumers covers all producers, including future ones, at almost no
cost. NIC drivers are not affected.

We can still add it later as hardening, if another class of bugs shows up.


>
> The few mirred/redirect cases are a bane, causing so many issues. And
> escape any such check, as they call netif_receive_skb, netif_rx,
> dev_queue_xmit, directly.
>
> If it's only act_mirred, bpf_redirect and OVS, maybe a single
> validation helper across all three before they pass to the rx/tx
> path, that for starters calls eth_type_trans_careful. It quite likely
> won't be the last packet corruption they are capable of..

I agree these are the problems, and Jamal just found another one in
__bpf_redirect_neigh(), which does its own skb_pull(ETH_HLEN).

A single helper is tempting, but what it must check depends on the
target:
- Egress to an Ethernet device: ETH_HLEN at skb_mac_header().
- Egress to an L3 device (tun, ipip, wireguard...): no MAC header at all.
- Ingress: data at the network header. eth_type_trans() is not
  called again, except through __dev_forward_skb2() (patch 3).

Also, the original loopback crash needs no redirect at all:
"vlan pop_eth" on lo egress is enough.

So I would keep this series as the minimal fix for net/stable, and
look at a common helper for net-next, probably based on
dev->min_header_len / dev->hard_header_len of the target device,
called from tcf_mirred_forward(), skb_do_redirect() and OVS do_output().

Thanks!

>
> >
> > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
> >   header first and returns a drop reason on failure.
> > - Patch 2 uses it in loopback_xmit().
> > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
> >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
> >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
> >   consuming the inner Ethernet header. This is no longer a crash after
> >   patch 2, but egress decode never delivered a correct frame.
> >
> > Other eth_type_trans() users already pull the header or never see
> > such skbs:
> > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
> >   before vrf_local_xmit().
> > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
> >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
> >   only trims frags.
> >
> > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> > all four cases crash in eth_type_trans():
> > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> > - lo egress "action vlan pop_eth continue", then a BPF program doing
> >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
> >   eth/mpls/eth/4096 bytes frame
> > With the series, no crash. An ICMP echo inside an IFE frame decoded
> > on lo egress is now received and answered. Normal traffic on lo,
> > veth (including 8000 byte pings) and vrf is unchanged.
> >
> > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> >
> > Eric Dumazet (4):
> >   net: add eth_type_trans_careful()
> >   loopback: use eth_type_trans_careful()
> >   net: use eth_type_trans_careful() in __dev_forward_skb2()
> >   openvswitch: use eth_type_trans_careful() in internal_dev_recv()
> >
> > Jamal Hadi Salim (1):
> >   net/sched: act_ife: do not consume the inner header on egress
> >
> >  drivers/net/loopback.c               |  8 +++++++-
> >  include/linux/etherdevice.h          | 28 ++++++++++++++++++++++++++++
> >  net/core/dev.c                       | 18 +++++++++++++-----
> >  net/openvswitch/vport-internal_dev.c |  8 +++++++-
> >  net/sched/act_ife.c                  |  4 ++++
> >  5 files changed, 59 insertions(+), 7 deletions(-)
> >
> > --
> > 2.53.0
> >
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 15:49   ` Eric Dumazet
@ 2026-10-27 19:48     ` Jamal Hadi Salim
  2026-10-27 20:34       ` Eric Dumazet
  0 siblings, 1 reply; 21+ messages in thread
From: Jamal Hadi Salim @ 2026-10-27 19:48 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Willem de Bruijn, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Willem de Bruijn, Simon Horman, netdev, Andrew Lunn, Aaron Conole,
	Eelco Chaudron, Ilya Maximets, Jiri Pirko, dev, Ren Wei

On Tue, Oct 27, 2026 at 11:50 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le mar. 27 oct. 2026 à 15:40, Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> a écrit :
> >
> > Eric Dumazet wrote:
> > > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > > NIC drivers always provide this, and they should keep using the fast
> > > version, without an extra pskb_may_pull().
> > >
> > > Devices looping skbs from their transmit path back to their receive
> > > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > > the BUG() in __skb_pull().
> > >
> > > Instead of fixing every producer, or adding a check to all
> > > eth_type_trans() users, fix the few consumers exposed to this:
> >
> > Definitely a nice approach.
>
> Thanks!
>
> >
> > You previously suggested catching most of the TC modifications with a
> > single check in sch_handle_(e|in)gress. Is that no longer needed?
>
> Not for these crashes. The sch_handle_egress() check would only cover
> tc egress. It would miss OVS and the redirect paths you mention, and it
> would add a check for every packet going through clsact. Fixing the
> consumers covers all producers, including future ones, at almost no
> cost. NIC drivers are not affected.
>
> We can still add it later as hardening, if another class of bugs shows up.
>
>
> >
> > The few mirred/redirect cases are a bane, causing so many issues. And
> > escape any such check, as they call netif_receive_skb, netif_rx,
> > dev_queue_xmit, directly.
> >
> > If it's only act_mirred, bpf_redirect and OVS, maybe a single
> > validation helper across all three before they pass to the rx/tx
> > path, that for starters calls eth_type_trans_careful. It quite likely
> > won't be the last packet corruption they are capable of..
>
> I agree these are the problems, and Jamal just found another one in
> __bpf_redirect_neigh(), which does its own skb_pull(ETH_HLEN).

I didnt add my reviewed-by or tested-by because i wasnt sure  if you
are going to resend and include the patch i sent?

cheers,
jamal

> A single helper is tempting, but what it must check depends on the
> target:
> - Egress to an Ethernet device: ETH_HLEN at skb_mac_header().
> - Egress to an L3 device (tun, ipip, wireguard...): no MAC header at all.
> - Ingress: data at the network header. eth_type_trans() is not
>   called again, except through __dev_forward_skb2() (patch 3).
>
> Also, the original loopback crash needs no redirect at all:
> "vlan pop_eth" on lo egress is enough.
>
> So I would keep this series as the minimal fix for net/stable, and
> look at a common helper for net-next, probably based on
> dev->min_header_len / dev->hard_header_len of the target device,
> called from tcf_mirred_forward(), skb_do_redirect() and OVS do_output().
>
> Thanks!
>
> >
> > >
> > > - Patch 1 adds eth_type_trans_careful(), which pulls the Ethernet
> > >   header first and returns a drop reason on failure.
> > > - Patch 2 uses it in loopback_xmit().
> > > - Patch 3 uses it in __dev_forward_skb2(), used by BPF redirects to
> > >   ingress and by veth, macvlan, ipvlan, netdevsim and l2tp_eth.
> > > - Patch 4 uses it in OVS internal_dev_recv(), covering both the MPLS
> > >   (TEB) and NSH pop cases at once (only the MPLS case was tested).
> > > - Patch 5 (from Jamal) fixes act_ife decode on egress, which was
> > >   consuming the inner Ethernet header. This is no longer a crash after
> > >   patch 2, but egress decode never delivered a correct frame.
> > >
> > > Other eth_type_trans() users already pull the header or never see
> > > such skbs:
> > > - vrf_process_v{4,6}_outbound() already pull ETH_HLEN + IP header
> > >   before vrf_local_xmit().
> > > - tun already uses pskb_may_pull_reason(skb, ETH_HLEN).
> > > - XDP: bpf_xdp_adjust_head() and bpf_xdp_adjust_tail() keep at least
> > >   ETH_HLEN bytes in the linear part, and multi-buffer tail shrinking
> > >   only trims frags.
> > >
> > > Tested under virtme-ng with AF_PACKET reproducers. Without the series,
> > > all four cases crash in eth_type_trans():
> > > - lo egress "action vlan pop_eth", eth/eth/8192 bytes frame
> > > - lo egress "action ife decode pipe", IFE frame with 8192 bytes payload
> > > - lo egress "action vlan pop_eth continue", then a BPF program doing
> > >   bpf_skb_change_head(skb, 1, 0) + bpf_redirect(veth1, BPF_F_INGRESS)
> > > - OVS flow "pop_eth,pop_mpls(0x6558),0" to the internal port,
> > >   eth/mpls/eth/4096 bytes frame
> > > With the series, no crash. An ICMP echo inside an IFE frame decoded
> > > on lo egress is now received and answered. Normal traffic on lo,
> > > veth (including 8000 byte pings) and vrf is unchanged.
> > >
> > > Link: https://lore.kernel.org/netdev/20261005052249.1914367-2-weir@nebusec.ai/
> > > Link: https://lore.kernel.org/netdev/ACT-N6N4.v3.r0.20261023200137-4@mojatatu.com/
> > >
> > > Eric Dumazet (4):
> > >   net: add eth_type_trans_careful()
> > >   loopback: use eth_type_trans_careful()
> > >   net: use eth_type_trans_careful() in __dev_forward_skb2()
> > >   openvswitch: use eth_type_trans_careful() in internal_dev_recv()
> > >
> > > Jamal Hadi Salim (1):
> > >   net/sched: act_ife: do not consume the inner header on egress
> > >
> > >  drivers/net/loopback.c               |  8 +++++++-
> > >  include/linux/etherdevice.h          | 28 ++++++++++++++++++++++++++++
> > >  net/core/dev.c                       | 18 +++++++++++++-----
> > >  net/openvswitch/vport-internal_dev.c |  8 +++++++-
> > >  net/sched/act_ife.c                  |  4 ++++
> > >  5 files changed, 59 insertions(+), 7 deletions(-)
> > >
> > > --
> > > 2.53.0
> > >
> >
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 19:48     ` Jamal Hadi Salim
@ 2026-10-27 20:34       ` Eric Dumazet
  2026-10-27 21:20         ` Jamal Hadi Salim
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Dumazet @ 2026-10-27 20:34 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Willem de Bruijn, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Willem de Bruijn, Simon Horman, netdev, Andrew Lunn, Aaron Conole,
	Eelco Chaudron, Ilya Maximets, Jiri Pirko, dev, Ren Wei

Le mar. 27 oct. 2026 à 20:48, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> I didnt add my reviewed-by or tested-by because i wasnt sure  if you
> are going to resend and include the patch i sent?
>

I think you should send this patch as a standalone patch, it does not
depend on anything in the series.

Thanks!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 20:34       ` Eric Dumazet
@ 2026-10-27 21:20         ` Jamal Hadi Salim
  0 siblings, 0 replies; 21+ messages in thread
From: Jamal Hadi Salim @ 2026-10-27 21:20 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Willem de Bruijn, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Willem de Bruijn, Simon Horman, netdev, Andrew Lunn, Aaron Conole,
	Eelco Chaudron, Ilya Maximets, Jiri Pirko, dev, Ren Wei

On Tue, Oct 27, 2026 at 4:34 PM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le mar. 27 oct. 2026 à 20:48, Jamal Hadi Salim <jhs@mojatatu.com> a écrit :
> > I didnt add my reviewed-by or tested-by because i wasnt sure  if you
> > are going to resend and include the patch i sent?
> >
>
> I think you should send this patch as a standalone patch, it does not
> depend on anything in the series.
>

In that case, for this series.
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Tested-by: Jamal Hadi Salim <jhs@mojatatu.com>

I will send tommorow.

cheers,
jamal
> Thanks!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
                   ` (6 preceding siblings ...)
  2026-10-27 14:40 ` Willem de Bruijn
@ 2026-10-27 21:25 ` Jakub Kicinski
  2026-10-28  5:02   ` Eric Dumazet
  7 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2026-10-27 21:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Paolo Abeni, Willem de Bruijn, Simon Horman,
	netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron, Ilya Maximets,
	Jamal Hadi Salim, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

On Tue, 27 Oct 2026 09:25:59 +0100 Eric Dumazet wrote:
> eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> NIC drivers always provide this, and they should keep using the fast
> version, without an extra pskb_may_pull().
> 
> Devices looping skbs from their transmit path back to their receive
> path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> the BUG() in __skb_pull().
> 
> Instead of fixing every producer, or adding a check to all
> eth_type_trans() users, fix the few consumers exposed to this:

Bot says:

BPF CI (bpf-ci) is reporting a consistent regression with this patch
applied: every single job variant (x86-64/aarch64/s390x, gcc/llvm,
no-alu32/cpuv4/ASAN) fails the exact same selftest subtest:

  #135 empty_skb

Specifically, the "veth ETH_HLEN+1 packet ingress [redirect_ingress]"
case in tools/testing/selftests/bpf/prog_tests/empty_skb.c now fails:

  test_empty_skb:FAIL:ret: veth ETH_HLEN+1 packet ingress [redirect_ingress]
  unexpected ret: actual 1 != expected 0

That test redirects a minimal (ETH_HLEN+1-byte) skb to a veth's RX path
via bpf_clone_redirect(..., BPF_F_INGRESS), which goes through
dev_forward_skb() -> __dev_forward_skb2(). Before this change the skb
was forwarded successfully (ret == 0); after switching to
eth_type_trans_careful() it is now dropped (actual == 1, i.e.
NET_RX_DROP).

Could you take a look at whether the new drop-on-failure behavior in
__dev_forward_skb2() is intentional for this previously-accepted
ETH_HLEN+1 packet, and if so, update the empty_skb selftest's expected
return value alongside this change? Right now the selftest and the new
kernel behavior disagree, and it's failing 100% of the time in CI on
every arch/toolchain/config combination we run.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack
  2026-10-27 21:25 ` Jakub Kicinski
@ 2026-10-28  5:02   ` Eric Dumazet
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Dumazet @ 2026-10-28  5:02 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, Paolo Abeni, Willem de Bruijn, Simon Horman,
	netdev, Andrew Lunn, Aaron Conole, Eelco Chaudron, Ilya Maximets,
	Jamal Hadi Salim, Jiri Pirko, dev, Willem de Bruijn, Ren Wei

Le mar. 27 oct. 2026 à 22:25, Jakub Kicinski <kuba@kernel.org> a écrit :
>
> On Tue, 27 Oct 2026 09:25:59 +0100 Eric Dumazet wrote:
> > eth_type_trans() assumes ETH_HLEN bytes are present in skb->head.
> > NIC drivers always provide this, and they should keep using the fast
> > version, without an extra pskb_may_pull().
> >
> > Devices looping skbs from their transmit path back to their receive
> > path get skbs built by the stack instead. tc actions (act_vlan pop_eth,
> > act_ife decode), BPF helpers (bpf_skb_change_head() before a redirect)
> > or OVS actions (pop_eth followed by pop_mpls/pop_nsh) can leave fewer
> > than ETH_HLEN bytes in the linear part, and eth_type_trans() then hits
> > the BUG() in __skb_pull().
> >
> > Instead of fixing every producer, or adding a check to all
> > eth_type_trans() users, fix the few consumers exposed to this:
>
> Bot says:
>
> BPF CI (bpf-ci) is reporting a consistent regression with this patch
> applied: every single job variant (x86-64/aarch64/s390x, gcc/llvm,
> no-alu32/cpuv4/ASAN) fails the exact same selftest subtest:
>
>   #135 empty_skb
>
> Specifically, the "veth ETH_HLEN+1 packet ingress [redirect_ingress]"
> case in tools/testing/selftests/bpf/prog_tests/empty_skb.c now fails:
>
>   test_empty_skb:FAIL:ret: veth ETH_HLEN+1 packet ingress [redirect_ingress]
>   unexpected ret: actual 1 != expected 0
>
> That test redirects a minimal (ETH_HLEN+1-byte) skb to a veth's RX path
> via bpf_clone_redirect(..., BPF_F_INGRESS), which goes through
> dev_forward_skb() -> __dev_forward_skb2(). Before this change the skb
> was forwarded successfully (ret == 0); after switching to
> eth_type_trans_careful() it is now dropped (actual == 1, i.e.
> NET_RX_DROP).
>
> Could you take a look at whether the new drop-on-failure behavior in
> __dev_forward_skb2() is intentional for this previously-accepted
> ETH_HLEN+1 packet, and if so, update the empty_skb selftest's expected
> return value alongside this change? Right now the selftest and the new
> kernel behavior disagree, and it's failing 100% of the time in CI on
> every arch/toolchain/config combination we run.

This test needs an update.

The failing program is the lwt_xmit one (redirect_ingress), not the tc
one. For lwt, BPF_PROG_TEST_RUN strips the 14-byte Ethernet header, so
the program sees a 1-byte skb, with skb->data at the network header.
bpf_clone_redirect(veth, BPF_F_INGRESS) then calls
eth_type_trans() on a 1-byte skb.

Before this patch, eth_type_trans() did not pull anything because
skb->len < ETH_HLEN, read the Ethernet header 13 bytes past skb->tail,
and sent a 1-byte "frame" up the stack. Now __dev_forward_skb2() drops
it and returns NET_RX_DROP (1).

The same packet sent with redirect_egress is already expected to fail
with 1, because veth_xmit() does pskb_may_pull(skb, ETH_HLEN) and drops it.
Ingress and egress now behave the same way. The tc variants (skb->len=15)
and the ipip variant (no MAC header, __bpf_rx_skb_no_mac()) are unchanged.

In v2, patch 3 will also update the selftest, renaming lwt_egress_ret
to lwt_ret so that it applies to both lwt programs. empty_skb passes
with it.

Thanks.

# ./test_progs -t empty_skb
[   50.888061] bpf_testmod: loading out-of-tree module taints kernel.
#115     empty_skb:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0/0 FAILED

diff --git a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
index c9fcb70cbafbce571d7826e233920193625dead1..a3938c16557b140e3ee5c116139cf8b636f85e35
100644
--- a/tools/testing/selftests/bpf/prog_tests/empty_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/empty_skb.c
@@ -24,7 +24,7 @@ void test_empty_skb(void)
                int *ifindex;
                int err;
                int ret;
-               int lwt_egress_ret; /* expected retval at lwt/egress */
+               int lwt_ret; /* expected retval at lwt */
                __be16 h_proto;
                bool success_on_tc;
                bool adjust_room;
@@ -82,7 +82,7 @@ void test_empty_skb(void)
                        .data_size_in = sizeof(eth_hlen),
                        .ifindex = &veth_ifindex,
                        .ret = -ERANGE,
-                       .lwt_egress_ret = -ERANGE,
+                       .lwt_ret = -ERANGE,
                        .success_on_tc = true,
                },
                {
@@ -96,17 +96,23 @@ void test_empty_skb(void)
                        .data_size_in = sizeof(eth_hlen),
                        .ifindex = &ipip_ifindex,
                        .ret = -ERANGE,
-                       .lwt_egress_ret = -ERANGE,
+                       .lwt_ret = -ERANGE,
                },

-               /* ETH_HLEN+1-sized packet should be redirected. */
+               /* ETH_HLEN+1-sized packet should be redirected at TC.
+                * At LWT, the Ethernet header is not part of the skb, and
+                * a 1-byte packet is too short for an Ethernet device.
+                */

                {
                        .msg = "veth ETH_HLEN+1 packet ingress",
                        .data_in = eth_hlen_pp,
                        .data_size_in = sizeof(eth_hlen_pp),
                        .ifindex = &veth_ifindex,
-                       .lwt_egress_ret = 1, /* veth_xmit NET_XMIT_DROP */
+                       /* lwt: skb->len=1, dropped by veth_xmit() at egress
+                        * and by __dev_forward_skb2() at ingress.
+                        */
+                       .lwt_ret = 1, /* NET_XMIT_DROP / NET_RX_DROP */
                },
                {
                        .msg = "ipip ETH_HLEN+1 packet ingress",
@@ -144,7 +150,6 @@ void test_empty_skb(void)
                        eth_hlen.h_proto = tests[i].h_proto;

                bpf_object__for_each_program(prog, bpf_obj->obj) {
-                       bool at_egress =
strstr(bpf_program__name(prog), "egress") != NULL;
                        bool at_tc =
!strncmp(bpf_program__section_name(prog), "tc", 2);
                        bool is_adjust_room =
!strcmp(bpf_program__name(prog), "tc_adjust_room");
                        int expected_ret;
@@ -153,7 +158,7 @@ void test_empty_skb(void)
                        if (tests[i].adjust_room != is_adjust_room)
                                continue;

-                       expected_ret = at_egress && !at_tc ?
tests[i].lwt_egress_ret : tests[i].ret;
+                       expected_ret = at_tc ? tests[i].ret : tests[i].lwt_ret;

                        tattr.data_in = tests[i].data_in;
                        tattr.data_size_in = tests[i].data_size_in;

Thanks.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-10-28  5:02 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27  8:25 [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Eric Dumazet
2026-10-27  8:26 ` [PATCH net 1/5] net: add eth_type_trans_careful() Eric Dumazet
2026-10-27 13:51   ` Aaron Conole
2026-10-27 15:02   ` Willem de Bruijn
2026-10-27  8:26 ` [PATCH net 2/5] loopback: use eth_type_trans_careful() Eric Dumazet
2026-10-27  8:26 ` [PATCH net 3/5] net: use eth_type_trans_careful() in __dev_forward_skb2() Eric Dumazet
2026-10-27  8:26 ` [PATCH net 4/5] openvswitch: use eth_type_trans_careful() in internal_dev_recv() Eric Dumazet
2026-10-27 13:50   ` Aaron Conole
2026-10-27  8:26 ` [PATCH net 5/5] net/sched: act_ife: do not consume the inner header on egress Eric Dumazet
2026-10-27  9:17 ` [PATCH net 0/5] net: fix eth_type_trans() crashes on skbs coming from the stack Jamal Hadi Salim
2026-10-27  9:51   ` Eric Dumazet
2026-10-27 13:07     ` Jamal Hadi Salim
2026-10-27 14:00       ` Aaron Conole
2026-10-27 14:33       ` Eric Dumazet
2026-10-27 14:40 ` Willem de Bruijn
2026-10-27 15:49   ` Eric Dumazet
2026-10-27 19:48     ` Jamal Hadi Salim
2026-10-27 20:34       ` Eric Dumazet
2026-10-27 21:20         ` Jamal Hadi Salim
2026-10-27 21:25 ` Jakub Kicinski
2026-10-28  5:02   ` Eric Dumazet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.