[PATCH net v4] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()

[PATCH net v4] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()

[Anton Danilov]

ip6gre_tunnel_xmit() and ip6erspan_tunnel_xmit() check the packet length
with pskb_inet_may_pull(), which looks at skb->protocol only, while the
code that follows parses the packet through VLAN tags with
skb_protocol(skb, true). The parsing started to look through the tags
with the two commits in the Fixes tags; the check was left as it was.
A VLAN-tagged frame whose inner IPv4/IPv6 header is not in the linear
area passes the check, and the inner header is read anyway. A 20 byte
tagged frame on ip6gretap and a 10 byte frame on ip6erspan are sent
out, while an untagged frame that is too short for its IP header is
already dropped.

Use skb_vlan_inet_prepare(), as the IPv6 receive path does since
commit 81c734dae203 ("ip6_tunnel: use skb_vlan_inet_prepare() in
__ip6_tnl_rcv()"). The second argument (inner_proto_inherit) depends on
the device: ip6gre_tunnel_xmit() serves both ip6gre (ARPHRD_IP6GRE, no
MAC header) and ip6gretap (ARPHRD_ETHER), so it is
dev->type != ARPHRD_ETHER there; ip6erspan is always an Ethernet device,
so it is false. vxlan does the same with no_eth_encap.

The tag walk starts at skb->mac_len - VLAN_HLEN, or at ETH_HLEN if
skb->mac_len is 0, and on transmit skb->mac_len is still what the skb
was received with. For a packet that came in through an NBMA gre
device and is routed out of a VLAN on ip6gretap or ip6erspan, the walk
starts inside the inner IPv4 header. Clear skb->mac_len for Ethernet
devices first.

An ip6gre device created without a remote uses ip6gre_header_ops, and
ip6gre_header() pushes a pseudo IPv6 header in front of the packet, so
there skb->data is not where the packet starts. skb_vlan_inet_prepare()
would move the network header onto that pseudo header, whose payload
length and GRE words are never written, and an ICMPv6 error for the
packet then quotes them: KMSAN reports uninit-value in
icmpv6_push_pending_frames(). Keep pskb_inet_may_pull() for that case,
as before this change.

With this change the two short frames above are dropped. gre_gso.sh,
l2_tos_ttl_inherit.sh and the mirror_gre, mirror_gre_vlan,
mirror_gre_bridge_1q and mirror_gre_changes forwarding selftests pass.
A constant true instead breaks the ip6gretap cases of mirror_gre.sh,
and a constant false breaks the ip6gre GSO cases of gre_gso.sh.

Fixes: 3f8a8447fd0b ("ip6_gre: use actual protocol to select xmit")
Fixes: b09ab9c92e50 ("ip6_tunnel: allow to inherit from VLAN encapsulated IP")
Reported-by: syzbot+6023ea32e206eef7920a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6023ea32e206eef7920a
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

---

v4: Anton Danilov takes over the patch, as Eric suggested in the v3 thread.
 - inner_proto_inherit follows the device type instead of being a
   constant.
 - Clear skb->mac_len for Ethernet devices before the VLAN walk.
 - Keep pskb_inet_may_pull() for ip6gre with header_ops.
 - Fixes tags point at the commits that made the parsing look through
   VLAN tags, instead of d8a6213d70ac ("geneve: fix header validation
   in geneve[6]_xmit_skb"), which only added the helper.
 - Discussion: https://lore.kernel.org/netdev/20261003220513.107668-1-littlesmilingcloud@gmail.com/
v3: https://lore.kernel.org/netdev/20260119112512.28196-1-fw@strlen.de/
v2: https://lore.kernel.org/netdev/20260106144529.1424886-1-edumazet@google.com/
v1: https://lore.kernel.org/netdev/20260105100330.2258612-1-edumazet@google.com/

 net/ipv6/ip6_gre.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

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

Re: [PATCH net v4] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 (2) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The check/parse mismatch the commit says it fixes is still
  reachable in ip6gre_tunnel_xmit() on ip6gre devices that have…
- [High] One of the parses this patch is meant to protect is the one
  added by Fixes: b09ab9c92e50, i.e. payload_protocol =…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the IPv4 GRE
  transmit paths have the same check/parse mismatch, and the patch…
- [Medium] This problem wasn't introduced by this patch, but on
  ip6erspan, frames that are not resolved to IPv4/IPv6 pass the new…

--

ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()

This patch replaces pskb_inet_may_pull() with skb_vlan_inet_prepare() in
ip6gre_tunnel_xmit() and ip6erspan_tunnel_xmit(). Before the check it
clears skb->mac_len on Ethernet devices. pskb_inet_may_pull() is still
used for ip6gre devices that have header_ops.

Closes: https://syzkaller.appspot.com/bug?extid=6023ea32e206eef7920a

> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index e61cb10b50dc9..f48141820fb5a 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;

[Severity: High]
Can the VLAN check/parse mismatch still be reached on this branch?

ip6gre_tunnel_init() installs header_ops only when the device is created
without a remote:

    if (ipv6_addr_any(&tunnel->parms.raddr))
        dev->header_ops = &ip6gre_header_ops;

Nothing clears it later. If ip6gre_changelink()->ip6gre_tnl_change() then
sets a remote, ip6gre_tnl_link_config_common() recomputes
IP6_TNL_F_CAP_XMIT. ip6_tnl_xmit_ctl() then passes, and packets still take
this branch.

When skb->protocol == ETH_P_8021Q, pskb_inet_may_pull() uses nhlen = 0 and
checks nothing. The dispatch further down still does:

    payload_protocol = skb_protocol(skb, true);

With mac_len == 0, that walk reads a vlan_hdr at skb->data + ETH_HLEN. That
location is inside the pseudo IPv6 header that ip6gre_header() pushed. The
encapsulated protocol it returns is saddr bytes 8-9, and those come from
t->parms.laddr:

    ipv6h->saddr = t->parms.laddr;

So if the user picks a laddr such as 2001:db8:0:0:86dd::1, the walk returns
ETH_P_IPV6.

One way to get there is AF_PACKET SOCK_DGRAM with sll_protocol set to
htons(ETH_P_8021Q), a non-zero sll_addr and an empty payload.
packet_snd() resets the network header before dev_hard_header(), so the
network header points at the empty payload. ip6gre_xmit_ipv6() then reads
past skb->tail:

    if (ipv6_addr_equal(&t->parms.raddr, &ipv6h->saddr))
        return -1;

prepare_ip6gre_xmit_ipv6() also reads nexthdr, dsfield and flowlabel from
the same unpulled bytes. If the protocol is ETH_P_IP instead,
prepare_ip6gre_xmit_ipv4() reads the tos.

KMSAN would report uninit-value here. With USE_ORIG_TCLASS or
USE_ORIG_FLOWLABEL, the stale bytes can be copied into the outer header.

There is a separate case on the same kind of device. When it receives a
frame through tc mirred or bpf redirect, ip6gre_header() is skipped, and
__gre6_xmit() reads this without any length guarantee:

    if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
        fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;

NBMA devices that still have no remote don't seem to be affected.
ip6_tnl_get_cap() returns only CAP_PER_PACKET when raddr is any, so
ip6_tnl_xmit_ctl() rejects them.

The commit message names this mismatch in ip6gre_tunnel_xmit(). Does this
branch need a VLAN-aware check measured from the existing network offset?
Alternatively, should VLAN ethertypes not be treated as IP here?

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

[Severity: High]
Does clearing mac_len here match the walk in ip6_tnl_xmit() that the second
Fixes tag (b09ab9c92e50) added?

That walk runs after the tunnel headers have been pushed:

__gre6_xmit()
    gre_build_header()
        skb_push(skb, hdr_len);
    ip6_tnl_xmit()
        payload_protocol = skb_protocol(skb, true);

skb_push() moves skb->data but does not change skb->protocol or mac_len.
With mac_len now 0, __vlan_get_protocol_offset() reads its vlan_hdr at
skb->data + ETH_HLEN, counted from the GRE header. skb_vlan_inet_prepare()
checked the Ethernet frame instead.

With a 4-byte GRE header the read happens to land on the TPID. In the
other cases it reads MAC address bytes:

  - tun_hlen 8 (key or seq): source MAC bytes 2-3
  - tun_hlen 12: destination MAC bytes 4-5
  - ip6erspan, after 16 pushed bytes: destination MAC bytes 0-1

Take ip6gretap with a key and an 18-byte VLAN-tagged ARP frame.
skb_vlan_inet_prepare() pulls 18 bytes and lets it through. If the source
MAC is xx:xx:08:00:xx:xx, ip6_tnl_xmit() treats payload_protocol as
ETH_P_IP.

hop_limit is 0 by default over netlink when IFLA_GRE_TTL is omitted, and
also for a collect_md key with ttl 0. In that case ip6_tnl_xmit() does:

    if (hop_limit == 0) {
        if (payload_protocol == htons(ETH_P_IP))
            hop_limit = ip_hdr(skb)->ttl;

This reads past the end of the frame and puts that byte in the outer IPv6
header.

The same applies to ip6erspan_tunnel_xmit(). There, the ip6_tnl_xmit()
walk is the only VLAN-aware parse after the check. So the base described
in the new comment ("The VLAN tag walks below start at ...") doesn't seem
to hold for it.

The NBMA daddr parsing in ip6_tnl_xmit() can't be reached here.
ip6_tnl_xmit_ctl() drops raddr == any first, and collect_md skips that
block.

Should the payload protocol be computed before the header push, or taken
from the network header that skb_vlan_inet_prepare() set?

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

[Severity: Medium]
This is a pre-existing issue, but on ip6erspan, frames that don't resolve
to IPv4 or IPv6 still get through with only ETH_HLEN bytes guaranteed.

skb_vlan_inet_prepare(skb, false) starts with maclen = ETH_HLEN and adds
nhlen only for IP. For a VLAN-tagged frame of 14 to 17 bytes,
__vlan_get_protocol_offset() fails in skb_header_pointer(), and
vlan_get_protocol_offset_inline() leaves depth unset:

    if (depth && res.type)
        *depth = res.depth;

So pskb_may_pull(14) succeeds, and the network header is set at offset 14.

ip6erspan_tunnel_xmit() then calls erspan_build_header() or
erspan_build_header_v2(), which do:

    tos = is_ipv4 ? ip_hdr(skb)->tos :
            (ipv6_hdr(skb)->priority << 4) +
            (ipv6_hdr(skb)->flow_lbl[0] >> 4);
    ...
    if (eth->h_proto == htons(ETH_P_8021Q)) {
        qp = (struct qtag_prefix *)(skb->data + 2 * ETH_ALEN);
        vlan_tci = ntohs(qp->tci);

Both reads hit skb->data[14..15], past skb->len, and the stale bytes are
sent in the ERSPAN COS and VLAN fields. The tos read also happens for an
untagged 14-byte non-IP frame.

This patch is still an improvement here: before it, the 10-byte frame
from the commit message was not dropped either.

Should ip6erspan_tunnel_xmit() require enough bytes for these reads in
erspan_build_header()?

[ ... ]

[Severity: Medium]
This isn't a bug introduced by this patch, but do the IPv4 GRE transmit
paths have the same check/parse mismatch?

ipgre_xmit(), erspan_xmit() and gre_tap_xmit() in net/ipv4/ip_gre.c still
begin with:

    if (!pskb_inet_may_pull(skb))
        goto free_skb;

That check uses nhlen = 0 for ETH_P_8021Q and ETH_P_8021AD.
ip_tunnel_xmit() then does payload_protocol = skb_protocol(skb, true). If
the walk returns IPv4 or IPv6, it reads
inner_iph = skb_inner_network_header(skb). By then
gre_handle_offloads()->iptunnel_handle_offloads()->skb_reset_inner_headers()
has made that equal to the network header.

The fields it reads include tos (tos inherit), frag_off (unless ignore_df),
ttl or hop_limit (ttl inherit) and, on NBMA, daddr:

    if (payload_protocol == htons(ETH_P_IP) && !tunnel->ignore_df)
        df |= (inner_iph->frag_off & htons(IP_DF));
    ...
        if (payload_protocol == htons(ETH_P_IP))
            ttl = inner_iph->ttl;

For example, send an 18-byte tagged frame with AF_PACKET SOCK_RAW on
gretap. packet_parse_headers() sets the network header at 18, so the
frag_off and ttl reads land past skb->len. Native IPv4 erspan v1 also
reads ip_hdr(skb)->tos in erspan_build_header().

The IPv4 side came from the IPv4 counterpart of b09ab9c92e50. Was ip_gre.c
looked at as part of this change, or would it need a similar follow-up?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261004145205.226974-1-littlesmilingcloud%40gmail.com

Re: [PATCH net v4] ip6_gre: use skb_vlan_inet_prepare() instead of pskb_inet_may_pull()

[Anton Danilov]

On Mon, Oct 05, 2026 at 02:55:11PM +0000, netdev-bot+sashiko@kernel.org wrote:

> - [High] The check/parse mismatch the commit says it fixes is still
>   reachable in ip6gre_tunnel_xmit() on ip6gre devices that have ...
> - [High] One of the parses this patch is meant to protect is the one
>   added by Fixes: b09ab9c92e50, i.e. payload_protocol = ...

Thanks for the review. I measured v4 against net on the cases it
points at - there is no regression. For a VLAN-tagged frame the outer
header stays the same where it was already wrong, and either starts to
inherit correctly or is now dropped where net let it through:

  tagged frame                         net        v4
  ip6gretap with key, ttl inherit      hlim 64    hlim 64  (unchanged)
  ip6erspan, ttl inherit               hlim 64    hlim 64  (unchanged)
  ip6gretap without key, forwarded     hlim 64    hlim 32  (now inherits)
  20B gretap / 10B erspan short frame  sent       dropped

High #1 (header_ops branch): that branch keeps pskb_inet_may_pull()
exactly as net has it, so the mismatch there is not introduced by this
patch. It is reachable only on an ip6gre device created without a
remote that is later given one with changelink. That belongs with the
rest of the ip6gre changelink/header_ops handling; v5 only narrows the
commit message so it no longer claims to cover that branch.

High #2 (the parse added by b09ab9c92e50): yes, ip6_tnl_xmit() walks
the tags again after gre_build_header() has pushed the GRE header, and
clearing mac_len does not help that walk -- the first two rows above
are unchanged from net for exactly that reason. That inheritance is a
separate fix that depends on this one, so it will come after. v5 fixes
the comment, which was meant to describe skb_vlan_inet_prepare()'s own
length check, not the ip6_tnl_xmit() parse.

The two Medium notes are pre-existing as well: the IPv4 paths
(gre_tap_xmit/erspan_xmit/ip_tunnel_rcv) and the erspan_build_header()
reads on short frames. Fixes for those are queued separately.

v5 changes only the commit message and two comments; the code is
identical to v4.

pw-bot: cr

---

Anton Danilov