All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <edumazet@google.com>
To: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com
Cc: horms@kernel.org, dsahern@kernel.org, idosch@nvidia.com,
	 netdev@vger.kernel.org, eric.dumazet@gmail.com,
	 Eric Dumazet <edumazet@google.com>
Subject: [PATCH net v2 0/5] ip_tunnel, ip_gre: fix changelink lengths and ERSPAN receive
Date: Wed, 16 Sep 2026 10:01:50 +0000	[thread overview]
Message-ID: <20260916100155.1398403-1-edumazet@google.com> (raw)
In-Reply-To: <20260912150944.3470971-1-edumazet@google.com>

This series fixes bugs in how IPv4/IPv6 tunnels and IPv4/IPv6 GRE/ERSPAN
tunnels apply a netlink changelink, and an out-of-bounds read in the ERSPAN
receive path. They all come from the same place: the changelink paths mutate
the live device as they go, without keeping tunnel->hlen,
dev->needed_headroom and dev->mtu in sync.

Patch 1 stops ip_tunnel_encap_setup() and ip6_tnl_encap_setup() from clearing
the active encapsulation before the requested one has been validated. They
memset() t->encap before calling ip[6]_encap_hlen(), so a request naming an
unknown encapsulation type, or one whose module is not loaded, returns -EINVAL
with FOU/GUE already switched off on a working tunnel, while t->encap_hlen and
t->hlen keep their old values. The memset() is redundant -- all four fields of
struct ip_tunnel_encap are assigned unconditionally once the length check has
passed, and a tunnel being created starts from the zeroed private area of
alloc_netdev() -- so it is simply removed. net-next already removed the IPv4
one in commit 88b84cae6b94 ("ip_tunnel: use WRITE_ONCE in
ip_tunnel_encap_setup"), which is not in net; removing the same lines here
keeps the merge trivial, and extends the fix to IPv6, which that commit did
not touch.

Patch 2 makes ipgre_netlink_parms() and erspan_netlink_parms() parse into
local variables and commit only once every attribute has been validated.
Today a request carrying IFLA_GRE_COLLECT_METADATA together with an invalid
IFLA_GRE_IGNORE_DF is rejected after dev->type has already become
ARPHRD_NONE, which breaks the interface for good.

Patch 3 stops maintaining the device lengths as a difference.
ipgre_link_update() adjusts them by a delta computed from tun_hlen only,
while ip_tunnel_bind_dev() assigns the same fields from tunnel->hlen. Two
writers, two models, and a delta that ignores the encapsulation, is applied
on top of the absolute assignment when the link changes too, and is computed
from a length ip_tunnel_encap_setup() may have published for a request that
then failed. tunnel->hlen is now recomputed from tun_hlen and encap_hlen, and
ip_tunnel_bind_dev() becomes the only writer of the device lengths. The patch
also restricts the dev->hard_header_len update to devices installing
ipgre_header_ops: ip_tunnel_bind_dev() subtracts hard_header_len from the MTU
only for ARPHRD_ETHER, so the old "if (dev->header_ops)" was inflating
gretap's 14-byte Ethernet header and having it subtracted a second time. Not
a memory safety issue: ip_tunnel_xmit() computes its own headroom for the
encapsulation, only the advertised MTU is wrong.

Patch 4 gives ERSPAN the same treatment, where erspan_tunnel_init() was the
only place computing tun_hlen and hlen even though erspan_changelink() can
change both. After a version 0 -> 2 change tun_hlen is still 4, so
gre_build_header() writes the sequence number at greh + tun_hlen - 4, that is
over greh->flags and greh->protocol, corrupting every transmitted packet.

Patch 5 fixes an out-of-bounds read of the ERSPAN metadata in collect_md
mode. erspan_rcv() and ip6erspan_rcv() copy 8 bytes from 12 bytes into the
GRE header, but only pull erspan_hdr_len(ver) bytes beyond it, which is 0
for version 0. A type I packet (4-byte GRE header, no ERSPAN header) reaches
a collect_md tunnel through itn->collect_md_tun and takes the
ERSPAN_V2_MDSIZE branch of the ternary, and a malformed packet with an 8-byte
GRE header and ershdr->ver == 0 gets there too, on both the IPv4 and the IPv6
side.

Apply order
-----------
Patch 1 must land before patch 3. ipgre_changelink() returns early, without
reaching the new link_update: label, when ipgre_newlink_encap_setup() fails,
which is only correct because patch 1 guarantees that a failing
ip_tunnel_encap_setup() has published nothing.

Behaviour change
----------------
Patches 3 and 4 recompute the MTU from ip_tunnel_bind_dev() instead of
shifting it by a delta, as ip_tunnel_update() already does for a link or
fwmark change. It is guarded by tunnel->hlen != old_hlen, so a MTU configured
by the user still survives a request that does not change the header length,
but one that does now discards it.

Patch 5 rejects an ERSPAN base header whose version is neither 1 nor 2,
where such packets were previously decapsulated. For version 0 the current
code leaves the 4-byte base header inside the payload, corrupting the inner
frame, and the transmit side (erspan_fb_xmit(), ip6erspan_tunnel_xmit())
already rejects anything that is not 1 or 2. Native ERSPAN type I tunnels
are not affected: they send a 4-byte GRE header and take the
is_erspan_type1() path.

Notes for reviewers, because not all bugs are fixed
---------------------------------------------------
The changelink paths still commit into the live tunnel step by step. A
rejected request is therefore not a no-op, and since the xmit path is
lockless, a concurrent erspan_xmit() can briefly see a new erspan_ver while
dev->needed_headroom still describes the old one. Patch 4 shrinks that window
from permanent to the duration of a single changelink, since
erspan_changelink() does not refresh the lengths at all today, but closing it
means publishing a whole new configuration atomically.

The same is true of __gre_xmit(), which takes a snapshot of the flags from
its caller but reads tunnel->tun_hlen live, while ipgre_changelink()
publishes t->parms.o_flags before ipgre_link_update() recomputes tun_hlen.
gre_build_header() walks backwards from greh + hdr_len - 4, so the two must
agree: new flags with an old tun_hlen writes the sequence number over
greh->flags and greh->protocol, and old flags with a new tun_hlen leaves the
four extra skb_push()ed bytes uninitialised on the wire.

Both are pre-existing and both need the larger rework, which will come with
the ip_tunnel RCU conversion in net-next.

v2:
  - Patch 1 is new: do not clear t->encap before the requested encapsulation
    has been validated, in the IPv4 and IPv6 tunnels. It must precede the
    former patch 2, now patch 3.
  - Patch 5 is new: fix the out-of-bounds read of the ERSPAN metadata in
    collect_md mode, on the IPv4 and IPv6 receive paths.
  - The three patches of v1 are unchanged in intent and become patches 2, 3
    and 4.
  - Patch 2 (was 1/3): clarify in the comments that only the attribute
    parsers become all-or-nothing, ip_tunnel_changelink() can still fail
    afterwards.
  - Patch 3 (was 2/3): test for ipgre_header_ops rather than for any
    header_ops, so gretap keeps its 14-byte hard_header_len; add a second
    Fixes tag for fdafed459998; drop the incorrect claim that
    ip_tunnel_xmit() cows before ip_tunnel_encap() pushes the FOU/GUE header,
    and describe instead the headroom ipgre_xmit() already reserved at tunnel
    entry.
  - Patch 4 (was 3/3): also refresh the lengths on the
    ip_tunnel_changelink() error paths; replace the skb_under_panic() claim,
    which is not reachable, with the actual GRE header corruption.
  - All five patches now carry Cc: stable@vger.kernel.org. Patch 4 does not
    build without patch 3.

Assisted-by: LLM
Link: https://lore.kernel.org/netdev/20260912150944.3470971-1-edumazet@google.com/ [v1]

Eric Dumazet (5):
  ip_tunnel: do not clear the active encap before validating the new one
  ip_gre: validate netlink attributes before changing the tunnel
  ip_gre: compute tunnel lengths absolutely instead of by delta
  ip_gre: recompute erspan header lengths after a change
  gre: fix out-of-bounds read of erspan metadata in collect_md mode

 include/net/ip_tunnels.h |   1 +
 net/ipv4/ip_gre.c        | 201 ++++++++++++++++++++++++++++-----------
 net/ipv4/ip_tunnel.c     |  19 +++-
 net/ipv6/ip6_gre.c       |   2 +
 net/ipv6/ip6_tunnel.c    |   2 -
 5 files changed, 164 insertions(+), 61 deletions(-)

-- 
2.55.0.1032.g73a4cd73de-goog


  parent reply	other threads:[~2026-09-16 10:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-12 15:09 [PATCH net 0/3] ip_gre: fix header lengths and validation on changelink Eric Dumazet
2026-09-12 15:09 ` [PATCH net 1/3] ip_gre: validate netlink attributes before changing the tunnel Eric Dumazet
2026-09-15 12:11   ` netdev-bot+sashiko
2026-09-12 15:09 ` [PATCH net 2/3] ip_gre: compute tunnel lengths absolutely instead of by delta Eric Dumazet
2026-09-15 12:11   ` netdev-bot+sashiko
2026-09-12 15:09 ` [PATCH net 3/3] ip_gre: recompute erspan header lengths after a change Eric Dumazet
2026-09-15 12:11   ` netdev-bot+sashiko
2026-09-15 13:31 ` [PATCH net 0/3] ip_gre: fix header lengths and validation on changelink Eric Dumazet
2026-09-16 10:01 ` Eric Dumazet [this message]
2026-09-16 10:01   ` [PATCH net v2 1/5] ip_tunnel: do not clear the active encap before validating the new one Eric Dumazet
2026-09-20 10:48     ` netdev-bot+sashiko
2026-09-16 10:01   ` [PATCH net v2 2/5] ip_gre: validate netlink attributes before changing the tunnel Eric Dumazet
2026-09-20 10:48     ` netdev-bot+sashiko
2026-09-16 10:01   ` [PATCH net v2 3/5] ip_gre: compute tunnel lengths absolutely instead of by delta Eric Dumazet
2026-09-20 10:48     ` netdev-bot+sashiko
2026-09-16 10:01   ` [PATCH net v2 4/5] ip_gre: recompute erspan header lengths after a change Eric Dumazet
2026-09-20 10:48     ` netdev-bot+sashiko
2026-09-16 10:01   ` [PATCH net v2 5/5] gre: fix out-of-bounds read of erspan metadata in collect_md mode Eric Dumazet
2026-09-20 10:48     ` netdev-bot+sashiko
2026-09-16 22:45   ` [PATCH net v2 0/5] ip_tunnel, ip_gre: fix changelink lengths and ERSPAN receive Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260916100155.1398403-1-edumazet@google.com \
    --to=edumazet@google.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=eric.dumazet@gmail.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.