* [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