Merge branch 'net-fix-eth_type_trans-crashes-on-skbs-coming-from-the-stack'

Eric Dumazet says:

====================
net: fix eth_type_trans() crashes on skbs coming from the stack

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/
v1: https://lore.kernel.org/netdev/20261027082604.1485903-1-edumazet@kernel.org/
====================

Link: https://patch.msgid.link/20261028122703.2093730-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-10-29 09:11:22 -07:00
commit c9897252a6
6 changed files with 71 additions and 14 deletions

View File

@ -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))

View File

@ -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,

View File

@ -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;
if (likely(!ret)) {
skb->protocol = eth_type_trans(skb, dev);
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
ret = ____dev_forward_skb(dev, skb, check_mtu);
if (unlikely(ret))
return ret;
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)

View File

@ -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);

View File

@ -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;
}

View File

@ -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;