Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
@ 2026-01-19 11:24 Florian Westphal
  2026-01-19 11:38 ` Eric Dumazet
  2026-01-19 17:06 ` Jakub Kicinski
  0 siblings, 2 replies; 7+ messages in thread
From: Florian Westphal @ 2026-01-19 11:24 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, syzbot+6023ea32e206eef7920a, kuba, davem, Eric Dumazet,
	Mazin Al Haddad, Florian Westphal

From: Eric Dumazet <edumazet@google.com>

I added skb_vlan_inet_prepare() helper in the cited commit, hinting
that we would need to use it more broadly.

syzbot confirmed this was the case in ip6_gre.

uninit-value in ip6table_mangle_hook+0x97d/0x9c0 net/ipv6/netfilter/ip6table_mangle.c:72
 ip6t_mangle_out net/ipv6/netfilter/ip6table_mangle.c:56 [inline]
 ip6table_mangle_hook+0x97d/0x9c0 net/ipv6/netfilter/ip6table_mangle.c:72
 nf_hook_entry_hookfn include/linux/netfilter.h:154 [inline]
 nf_hook_slow+0xf4/0x400 net/netfilter/core.c:626
 nf_hook include/linux/netfilter.h:269 [inline]
 __ip6_local_out+0x5ac/0x640 net/ipv6/output_core.c:143
 ip6_local_out+0x4c/0x210 net/ipv6/output_core.c:153
 ip6tunnel_xmit+0x129/0x460 include/net/ip6_tunnel.h:161
 ip6_tnl_xmit+0x341a/0x3860 net/ipv6/ip6_tunnel.c:1281

Uninit was stored to memory at:
 ip6_tnl_xmit+0x34f7/0x3860 net/ipv6/ip6_tunnel.c:1277
 __gre6_xmit+0x14b9/0x1550 net/ipv6/ip6_gre.c:815
 ip6gre_xmit_ipv4 net/ipv6/ip6_gre.c:839 [inline]
 ip6gre_tunnel_xmit+0x18f7/0x2030 net/ipv6/ip6_gre.c:922

Uninit was created at:
 slab_post_alloc_hook mm/slub.c:4091 [inline]
 slab_alloc_node mm/slub.c:4134 [inline]
 __do_kmalloc_node mm/slub.c:4263 [inline]
 __kmalloc_node_track_caller_noprof+0x6c7/0xf90 mm/slub.c:4283
 kmalloc_reserve+0x23e/0x4a0 net/core/skbuff.c:609
 pskb_expand_head+0x226/0x1a60 net/core/skbuff.c:2275
 skb_realloc_headroom+0x140/0x2b0 net/core/skbuff.c:2355
 ip6_tnl_xmit+0x2106/0x3860 net/ipv6/ip6_tunnel.c:1227
 __gre6_xmit+0x14b9/0x1550 net/ipv6/ip6_gre.c:815
 ip6gre_xmit_ipv4 net/ipv6/ip6_gre.c:839 [inline]
 ip6gre_tunnel_xmit+0x18f7/0x2030 net/ipv6/ip6_gre.c:922

Fixes: d8a6213d70ac ("geneve: fix header validation in geneve[6]_xmit_skb")
Reported-by: syzbot+6023ea32e206eef7920a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6023ea32e206eef7920a
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Mazin Al Haddad <mazin@getstate.dev>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v3: pass 'true' argument to skb_vlan_inet_prepare to not change network
 header offset.

 net/ipv6/ip6_gre.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index d19d86ed4376..9e214c355e6c 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -881,7 +881,7 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
 	__be16 payload_protocol;
 	int ret;
 
-	if (!pskb_inet_may_pull(skb))
+	if (skb_vlan_inet_prepare(skb, true))
 		goto tx_err;
 
 	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
@@ -929,7 +929,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 	__u32 mtu;
 	int nhoff;
 
-	if (!pskb_inet_may_pull(skb))
+	if (skb_vlan_inet_prepare(skb, true))
 		goto tx_err;
 
 	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
-- 
2.52.0


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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-01-19 11:24 [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull() Florian Westphal
@ 2026-01-19 11:38 ` Eric Dumazet
  2026-01-19 17:06 ` Jakub Kicinski
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-01-19 11:38 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netdev, pabeni, syzbot+6023ea32e206eef7920a, kuba, davem,
	Mazin Al Haddad

On Mon, Jan 19, 2026 at 12:25 PM Florian Westphal <fw@strlen.de> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> I added skb_vlan_inet_prepare() helper in the cited commit, hinting
> that we would need to use it more broadly.
>
> syzbot confirmed this was the case in ip6_gre.
>
> uninit-value in ip6table_mangle_hook+0x97d/0x9c0 net/ipv6/netfilter/ip6table_mangle.c:72
>  ip6t_mangle_out net/ipv6/netfilter/ip6table_mangle.c:56 [inline]
>  ip6table_mangle_hook+0x97d/0x9c0 net/ipv6/netfilter/ip6table_mangle.c:72
>  nf_hook_entry_hookfn include/linux/netfilter.h:154 [inline]
>  nf_hook_slow+0xf4/0x400 net/netfilter/core.c:626
>  nf_hook include/linux/netfilter.h:269 [inline]
>  __ip6_local_out+0x5ac/0x640 net/ipv6/output_core.c:143
>  ip6_local_out+0x4c/0x210 net/ipv6/output_core.c:153
>  ip6tunnel_xmit+0x129/0x460 include/net/ip6_tunnel.h:161
>  ip6_tnl_xmit+0x341a/0x3860 net/ipv6/ip6_tunnel.c:1281
>
> Uninit was stored to memory at:
>  ip6_tnl_xmit+0x34f7/0x3860 net/ipv6/ip6_tunnel.c:1277
>  __gre6_xmit+0x14b9/0x1550 net/ipv6/ip6_gre.c:815
>  ip6gre_xmit_ipv4 net/ipv6/ip6_gre.c:839 [inline]
>  ip6gre_tunnel_xmit+0x18f7/0x2030 net/ipv6/ip6_gre.c:922
>
> Uninit was created at:
>  slab_post_alloc_hook mm/slub.c:4091 [inline]
>  slab_alloc_node mm/slub.c:4134 [inline]
>  __do_kmalloc_node mm/slub.c:4263 [inline]
>  __kmalloc_node_track_caller_noprof+0x6c7/0xf90 mm/slub.c:4283
>  kmalloc_reserve+0x23e/0x4a0 net/core/skbuff.c:609
>  pskb_expand_head+0x226/0x1a60 net/core/skbuff.c:2275
>  skb_realloc_headroom+0x140/0x2b0 net/core/skbuff.c:2355
>  ip6_tnl_xmit+0x2106/0x3860 net/ipv6/ip6_tunnel.c:1227
>  __gre6_xmit+0x14b9/0x1550 net/ipv6/ip6_gre.c:815
>  ip6gre_xmit_ipv4 net/ipv6/ip6_gre.c:839 [inline]
>  ip6gre_tunnel_xmit+0x18f7/0x2030 net/ipv6/ip6_gre.c:922
>
> Fixes: d8a6213d70ac ("geneve: fix header validation in geneve[6]_xmit_skb")
> Reported-by: syzbot+6023ea32e206eef7920a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6023ea32e206eef7920a
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Mazin Al Haddad <mazin@getstate.dev>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  v3: pass 'true' argument to skb_vlan_inet_prepare to not change network
>  header offset.

Ah right, this escaped my radar, thanks Florian !

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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-01-19 11:24 [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull() Florian Westphal
  2026-01-19 11:38 ` Eric Dumazet
@ 2026-01-19 17:06 ` Jakub Kicinski
  2026-01-19 17:17   ` Eric Dumazet
  1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-01-19 17:06 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netdev, pabeni, syzbot+6023ea32e206eef7920a, davem, Eric Dumazet,
	Mazin Al Haddad

On Mon, 19 Jan 2026 12:24:57 +0100 Florian Westphal wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> I added skb_vlan_inet_prepare() helper in the cited commit, hinting
> that we would need to use it more broadly.

I _think_ this makes GRE forwarding tests a bit unhappy:

https://netdev.bots.linux.dev/contest.html?branch=net-next-2026-01-19--12-00&executor=vmksft-forwarding&pw-n=0&pass=0
-- 
pw-bot: cr

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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-01-19 17:06 ` Jakub Kicinski
@ 2026-01-19 17:17   ` Eric Dumazet
  2026-01-20  0:01     ` Florian Westphal
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2026-01-19 17:17 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Florian Westphal, netdev, pabeni, syzbot+6023ea32e206eef7920a,
	davem, Mazin Al Haddad

On Mon, Jan 19, 2026 at 6:06 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 19 Jan 2026 12:24:57 +0100 Florian Westphal wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > I added skb_vlan_inet_prepare() helper in the cited commit, hinting
> > that we would need to use it more broadly.
>
> I _think_ this makes GRE forwarding tests a bit unhappy:
>
> https://netdev.bots.linux.dev/contest.html?branch=net-next-2026-01-19--12-00&executor=vmksft-forwarding&pw-n=0&pass=0
> --

I was unsure about ip6erspan_tunnel_xmit() change, I think I started
full tests days ago but probably was distracted.

I had :

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index d19d86ed43766bbc8ec052113be02ab231a5272c..9e214c355e6ce15fa828866ae20fa8fe321b4bf7
100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -881,7 +881,7 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
        __be16 payload_protocol;
        int ret;

-       if (!pskb_inet_may_pull(skb))
+       if (skb_vlan_inet_prepare(skb, true))
                goto tx_err;

        if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
@@ -929,7 +929,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct
sk_buff *skb,
        __u32 mtu;
        int nhoff;

-       if (!pskb_inet_may_pull(skb))
+       if (skb_vlan_inet_prepare(skb, false))
                goto tx_err;

        if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))

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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-01-19 17:17   ` Eric Dumazet
@ 2026-01-20  0:01     ` Florian Westphal
  2026-10-03 22:05       ` Anton Danilov
  0 siblings, 1 reply; 7+ messages in thread
From: Florian Westphal @ 2026-01-20  0:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jakub Kicinski, netdev, pabeni, syzbot+6023ea32e206eef7920a,
	davem, Mazin Al Haddad

Eric Dumazet <edumazet@google.com> wrote:
> On Mon, Jan 19, 2026 at 6:06 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Mon, 19 Jan 2026 12:24:57 +0100 Florian Westphal wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > I added skb_vlan_inet_prepare() helper in the cited commit, hinting
> > > that we would need to use it more broadly.
> >
> > I _think_ this makes GRE forwarding tests a bit unhappy:
> >
> > https://netdev.bots.linux.dev/contest.html?branch=net-next-2026-01-19--12-00&executor=vmksft-forwarding&pw-n=0&pass=0
> > --
> 
> I was unsure about ip6erspan_tunnel_xmit() change, I think I started
> full tests days ago but probably was distracted.
> 
> I had :
> 
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index d19d86ed43766bbc8ec052113be02ab231a5272c..9e214c355e6ce15fa828866ae20fa8fe321b4bf7
> 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -881,7 +881,7 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
>         __be16 payload_protocol;
>         int ret;
> 
> -       if (!pskb_inet_may_pull(skb))
> +       if (skb_vlan_inet_prepare(skb, true))
>                 goto tx_err;

It has to be either true or false depending on test case 8-/

gre_gso.sh needs this to be set to true, skbs don't have a mac
header: with "false": skb nhoff gets munged from 0 to 14.

But in mirror_gre.sh test case, skbs do have a mac header:
"true" munges nh offset from 14 to 0 and test fails.

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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-01-20  0:01     ` Florian Westphal
@ 2026-10-03 22:05       ` Anton Danilov
  2026-10-04  7:10         ` Eric Dumazet
  0 siblings, 1 reply; 7+ messages in thread
From: Anton Danilov @ 2026-10-03 22:05 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Eric Dumazet, Jakub Kicinski, netdev, Paolo Abeni,
	David S . Miller, Simon Horman, David Ahern, Ido Schimmel,
	Mazin Al Haddad

On Tue, Jan 20, 2026 at 01:01:01AM +0100, Florian Westphal wrote:

> It has to be either true or false depending on test case 8-/
>
> gre_gso.sh needs this to be set to true, skbs don't have a mac
> header: with "false": skb nhoff gets munged from 0 to 14.
>
> But in mirror_gre.sh test case, skbs do have a mac header:
> "true" munges nh offset from 14 to 0 and test fails.

Sorry for reviving an old thread: the syzbot report is still open and
net still has pskb_inet_may_pull() in both functions.

The two tests use different devices behind the same
ip6gre_tunnel_xmit(): ip6gre (no MAC header) in gre_gso.sh and
ip6gretap (ARPHRD_ETHER) in mirror_gre.sh. So the argument can
follow the device type, as vxlan does with no_eth_encap:

	skb_vlan_inet_prepare(skb, dev->type != ARPHRD_ETHER)

ip6erspan is always ARPHRD_ETHER, so false there, as Eric had it.

Tested on net (6e0022b5ae3d), changing only ip6_gre.c:

  ip6gre, ip6erspan     gre_gso.sh          mirror_gre.sh
  --------------------------------------------------------------
  unpatched             pass                pass
  false, false (v2)     2 GSO tests fail    pass
  true,  true (v3)      pass                2 ip6gretap tests fail
  true,  false          pass                2 ip6gretap tests fail
  diff below            pass                pass

The selftests only check that normal traffic still goes. They don't
send short tagged frames, so they do not hit this bug. The bug needs
such a frame: a 20 byte tagged frame on ip6gretap and a 10 byte frame
on ip6erspan are sent out on net, and dropped with the diff. An untagged
frame that is too short for its IP header is already dropped by 
pskb_inet_may_pull(); the diff only applies the same rule to tagged
frames.

The diff also fixes two problems I found while testing:

1. On transmit, skb->mac_len is still that of the receiving device,
   and the VLAN walk starts from it. A packet that came in on an NBMA
   ipgre device (mac_len 24) and is routed to a VLAN on ip6gretap has
   its walk start inside the inner IPv4 header. On net the tag is
   then missed and nothing is inherited; with skb_vlan_inet_prepare()
   alone, a crafted packet also gets its traffic class from the wrong
   byte. The diff clears skb->mac_len for Ethernet devices.

2. ip6gre without a remote has header_ops, so skb->data points at the
   pseudo header from ip6gre_header(). With true, the network header
   lands on it, and an ICMPv6 error then quotes its unwritten bytes
   (KMSAN in icmpv6_push_pending_frames()). The diff keeps
   pskb_inet_may_pull() there, as in net.

With the diff, gre_gso, l2_tos_ttl_inherit and
mirror_gre{,_vlan,_bridge_1q,_changes} pass. KMSAN, run with my
other GRE fixes on top, shows nothing on these paths.

About Fixes: v3 blames d8a6213d70ac ("geneve: fix header validation
in geneve[6]_xmit_skb"). That commit only added
skb_vlan_inet_prepare(); the bug in ip6_gre is older.

The length check, pskb_inet_may_pull(), looks at skb->protocol. For
a tagged frame that is ETH_P_8021Q, so the inner IP header is not
checked at all. This used to be fine: the code after the check also
used skb->protocol, so tagged frames went to ip6gre_xmit_other(),
which does not read the inner header.

Two later commits made the code look through the tags, but did not
change the check:

 - 3f8a8447fd0b ("ip6_gre: use actual protocol to select xmit") made
   ip6gre_tunnel_xmit() switch on skb_protocol(skb, true). Tagged
   IPv4 and IPv6 frames now go to ip6gre_xmit_ipv4() and
   ip6gre_xmit_ipv6(), which read the inner header.

 - b09ab9c92e50 ("ip6_tunnel: allow to inherit from VLAN encapsulated
   IP") did the same in ip6_tnl_xmit(), which reads the inner header
   to inherit the hop limit. ip6erspan goes through this path too.

Since then a short tagged frame passes the check, and its inner
header is read from bytes that were never pulled into the linear
data. That is the uninit-value syzbot reports.

skb_vlan_inet_prepare() walks the tags the same way as
skb_protocol(skb, true), pulls the inner IP header and points the
network header at it. So the check now covers what the code reads
later. That is why I would point Fixes at the two commits above:

Fixes: 3f8a8447fd0b ("ip6_gre: use actual protocol to select xmit")
Fixes: b09ab9c92e50 ("ip6_tunnel: allow to inherit from VLAN encapsulated IP")

The IPv4 side has the same problems: gre_tap_xmit(), erspan_xmit()
and ip_tunnel_rcv(). ip6_tnl_xmit() has one more: it walks the tags
after the GRE header is pushed. I will send these fixes separately;
the ip6_tnl_xmit() one depends on this patch.

Eric, would you like to respin with this, or should I send it as v4?

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index e61cb10b50dc..f48141820fb5 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -883,8 +883,22 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
 	__be16 payload_protocol;
 	int ret;
 
-	if (!pskb_inet_may_pull(skb))
-		goto tx_err;
+	if (dev->type != ARPHRD_ETHER && dev->header_ops) {
+		/* ip6gre_header() has pushed a pseudo header in front of
+		 * the packet, so skb->data is not where the packet starts.
+		 */
+		if (!pskb_inet_may_pull(skb))
+			goto tx_err;
+	} else {
+		/* The VLAN tag walks below start at skb->mac_len - VLAN_HLEN,
+		 * or at ETH_HLEN if it is 0, and a forwarded skb still has
+		 * the mac_len of the device it was received on.
+		 */
+		if (dev->type == ARPHRD_ETHER)
+			skb->mac_len = 0;
+		if (skb_vlan_inet_prepare(skb, dev->type != ARPHRD_ETHER))
+			goto tx_err;
+	}
 
 	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
 		goto tx_err;
@@ -934,7 +948,12 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 	__u32 mtu;
 	int nhoff;
 
-	if (!pskb_inet_may_pull(skb))
+	/* The VLAN tag walks below start at skb->mac_len - VLAN_HLEN, or at
+	 * ETH_HLEN if it is 0, and a forwarded skb still has the mac_len of
+	 * the device it was received on.
+	 */
+	skb->mac_len = 0;
+	if (skb_vlan_inet_prepare(skb, false))
 		goto tx_err;
 
 	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))

-- 
Anton Danilov


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

* Re: [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()
  2026-10-03 22:05       ` Anton Danilov
@ 2026-10-04  7:10         ` Eric Dumazet
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-10-04  7:10 UTC (permalink / raw)
  To: Anton Danilov
  Cc: Florian Westphal, Jakub Kicinski, netdev, Paolo Abeni,
	David S . Miller, Simon Horman, David Ahern, Ido Schimmel,
	Mazin Al Haddad

Le dim. 4 oct. 2026 à 00:05, Anton Danilov
<littlesmilingcloud@gmail.com> a écrit :
>
> On Tue, Jan 20, 2026 at 01:01:01AM +0100, Florian Westphal wrote:
>
> > It has to be either true or false depending on test case 8-/
> >
> > gre_gso.sh needs this to be set to true, skbs don't have a mac
> > header: with "false": skb nhoff gets munged from 0 to 14.
> >
> > But in mirror_gre.sh test case, skbs do have a mac header:
> > "true" munges nh offset from 14 to 0 and test fails.
>
> Sorry for reviving an old thread: the syzbot report is still open and
> net still has pskb_inet_may_pull() in both functions.
>

Do not be sorry, thanks a lot for working on this.

Please send a V4 (and take full ownership of this patch now you made
significant progress)

Thanks again!

> The two tests use different devices behind the same
> ip6gre_tunnel_xmit(): ip6gre (no MAC header) in gre_gso.sh and
> ip6gretap (ARPHRD_ETHER) in mirror_gre.sh. So the argument can
> follow the device type, as vxlan does with no_eth_encap:
>
>         skb_vlan_inet_prepare(skb, dev->type != ARPHRD_ETHER)
>
> ip6erspan is always ARPHRD_ETHER, so false there, as Eric had it.
>
> Tested on net (6e0022b5ae3d), changing only ip6_gre.c:
>
>   ip6gre, ip6erspan     gre_gso.sh          mirror_gre.sh
>   --------------------------------------------------------------
>   unpatched             pass                pass
>   false, false (v2)     2 GSO tests fail    pass
>   true,  true (v3)      pass                2 ip6gretap tests fail
>   true,  false          pass                2 ip6gretap tests fail
>   diff below            pass                pass
>
> The selftests only check that normal traffic still goes. They don't
> send short tagged frames, so they do not hit this bug. The bug needs
> such a frame: a 20 byte tagged frame on ip6gretap and a 10 byte frame
> on ip6erspan are sent out on net, and dropped with the diff. An untagged
> frame that is too short for its IP header is already dropped by
> pskb_inet_may_pull(); the diff only applies the same rule to tagged
> frames.
>
> The diff also fixes two problems I found while testing:
>
> 1. On transmit, skb->mac_len is still that of the receiving device,
>    and the VLAN walk starts from it. A packet that came in on an NBMA
>    ipgre device (mac_len 24) and is routed to a VLAN on ip6gretap has
>    its walk start inside the inner IPv4 header. On net the tag is
>    then missed and nothing is inherited; with skb_vlan_inet_prepare()
>    alone, a crafted packet also gets its traffic class from the wrong
>    byte. The diff clears skb->mac_len for Ethernet devices.
>
> 2. ip6gre without a remote has header_ops, so skb->data points at the
>    pseudo header from ip6gre_header(). With true, the network header
>    lands on it, and an ICMPv6 error then quotes its unwritten bytes
>    (KMSAN in icmpv6_push_pending_frames()). The diff keeps
>    pskb_inet_may_pull() there, as in net.
>
> With the diff, gre_gso, l2_tos_ttl_inherit and
> mirror_gre{,_vlan,_bridge_1q,_changes} pass. KMSAN, run with my
> other GRE fixes on top, shows nothing on these paths.
>
> About Fixes: v3 blames d8a6213d70ac ("geneve: fix header validation
> in geneve[6]_xmit_skb"). That commit only added
> skb_vlan_inet_prepare(); the bug in ip6_gre is older.
>
> The length check, pskb_inet_may_pull(), looks at skb->protocol. For
> a tagged frame that is ETH_P_8021Q, so the inner IP header is not
> checked at all. This used to be fine: the code after the check also
> used skb->protocol, so tagged frames went to ip6gre_xmit_other(),
> which does not read the inner header.
>
> Two later commits made the code look through the tags, but did not
> change the check:
>
>  - 3f8a8447fd0b ("ip6_gre: use actual protocol to select xmit") made
>    ip6gre_tunnel_xmit() switch on skb_protocol(skb, true). Tagged
>    IPv4 and IPv6 frames now go to ip6gre_xmit_ipv4() and
>    ip6gre_xmit_ipv6(), which read the inner header.
>
>  - b09ab9c92e50 ("ip6_tunnel: allow to inherit from VLAN encapsulated
>    IP") did the same in ip6_tnl_xmit(), which reads the inner header
>    to inherit the hop limit. ip6erspan goes through this path too.
>
> Since then a short tagged frame passes the check, and its inner
> header is read from bytes that were never pulled into the linear
> data. That is the uninit-value syzbot reports.
>
> skb_vlan_inet_prepare() walks the tags the same way as
> skb_protocol(skb, true), pulls the inner IP header and points the
> network header at it. So the check now covers what the code reads
> later. That is why I would point Fixes at the two commits above:
>
> Fixes: 3f8a8447fd0b ("ip6_gre: use actual protocol to select xmit")
> Fixes: b09ab9c92e50 ("ip6_tunnel: allow to inherit from VLAN encapsulated IP")
>
> The IPv4 side has the same problems: gre_tap_xmit(), erspan_xmit()
> and ip_tunnel_rcv(). ip6_tnl_xmit() has one more: it walks the tags
> after the GRE header is pushed. I will send these fixes separately;
> the ip6_tnl_xmit() one depends on this patch.
>
> Eric, would you like to respin with this, or should I send it as v4?
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index e61cb10b50dc..f48141820fb5 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -883,8 +883,22 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb,
>         __be16 payload_protocol;
>         int ret;
>
> -       if (!pskb_inet_may_pull(skb))
> -               goto tx_err;
> +       if (dev->type != ARPHRD_ETHER && dev->header_ops) {
> +               /* ip6gre_header() has pushed a pseudo header in front of
> +                * the packet, so skb->data is not where the packet starts.
> +                */
> +               if (!pskb_inet_may_pull(skb))
> +                       goto tx_err;
> +       } else {
> +               /* The VLAN tag walks below start at skb->mac_len - VLAN_HLEN,
> +                * or at ETH_HLEN if it is 0, and a forwarded skb still has
> +                * the mac_len of the device it was received on.
> +                */
> +               if (dev->type == ARPHRD_ETHER)
> +                       skb->mac_len = 0;
> +               if (skb_vlan_inet_prepare(skb, dev->type != ARPHRD_ETHER))
> +                       goto tx_err;
> +       }
>
>         if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
>                 goto tx_err;
> @@ -934,7 +948,12 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
>         __u32 mtu;
>         int nhoff;
>
> -       if (!pskb_inet_may_pull(skb))
> +       /* The VLAN tag walks below start at skb->mac_len - VLAN_HLEN, or at
> +        * ETH_HLEN if it is 0, and a forwarded skb still has the mac_len of
> +        * the device it was received on.
> +        */
> +       skb->mac_len = 0;
> +       if (skb_vlan_inet_prepare(skb, false))
>                 goto tx_err;
>
>         if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))
>
> --
> Anton Danilov
>

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

end of thread, other threads:[~2026-10-04  7:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-19 11:24 [PATCH net v3] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull() Florian Westphal
2026-01-19 11:38 ` Eric Dumazet
2026-01-19 17:06 ` Jakub Kicinski
2026-01-19 17:17   ` Eric Dumazet
2026-01-20  0:01     ` Florian Westphal
2026-10-03 22:05       ` Anton Danilov
2026-10-04  7:10         ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox