[PATCH net v3 0/5] ip_tunnel, ip_gre: fix header length and validation bugs

[PATCH net v3 0/5] ip_tunnel, ip_gre: fix header length and validation bugs

[Eric Dumazet]

This series fixes netlink validation and header length accounting bugs
in ip_tunnel, ip6_tunnel, ip_gre, and ip6_gre:

1. Do not zero the active tunnel encapsulation in ip_tunnel_encap_setup()
   and ip6_tnl_encap_setup() before the new encapsulation has been
   validated.
2. Validate netlink attributes in ipgre_netlink_parms() and
   erspan_netlink_parms() into a temporary struct ip_gre_parm before
   mutating tunnel state, reject toggling IFLA_GRE_COLLECT_METADATA in
   changelink, and validate attributes before calling
   ipgre_newlink_encap_setup() in changelink.
3. Recompute GRE tunnel header lengths and MTU absolutely from tun_hlen
   and encap_hlen via ip_tunnel_refresh_lengths() rather than shifting by
   delta, fixing drift on encapsulation changes and hard_header_len
   corruption on gretap devices.
4. Recompute ERSPAN tunnel header lengths and MTU on changelink, reject
   non-zero GRE flags when IFLA_GRE_ERSPAN_VER is 0 or
   IFLA_GRE_COLLECT_METADATA is set, and ensure IP_TUNNEL_SEQ_BIT matches
   erspan_ver in erspan_xmit().
5. Reject malformed ERSPAN base header versions (ver != 1 && ver != 2)
   and skip ERSPAN metadata extraction for Type I frames in collect_md
   mode instead of copying inner Ethernet frame bytes into md->u.md2.

v3:
 - Patch 1: clarify commit message that callers still invoke
   ip[6]_tunnel_encap_setup() before validating the rest of changelink.
 - Patch 2: stage ignore_df, fwmark, erspan_ver, index, hwid, and dir into
   struct ip_gre_parm and commit after ip_tunnel_changelink() succeeds,
   reject toggling IFLA_GRE_COLLECT_METADATA in changelink, and parse
   netlink parms before ipgre_newlink_encap_setup() in changelink.
 - Patch 3: update old_hlen to the intermediate t->hlen when
   ip_tunnel_update() recomputes dev->mtu after a link or fwmark change so
   compensating encap + o_flags changes still refresh dev->mtu.
 - Patch 4: check IFLA_GRE_OFLAGS/IFLA_GRE_IFLAGS before the
   IFLA_GRE_ERSPAN_VER == 0 early return in erspan_validate(), ensure
   IP_TUNNEL_SEQ_BIT matches tunnel->erspan_ver in erspan_xmit(), and
   commit gparms + o_flags atomically after ip_tunnel_changelink() succeeds.
 - Patch 5: check tpi->proto == htons(ETH_P_ERSPAN) in is_erspan_type1(),
   and clarify commit message and comment that __iptunnel_pull_header()
   already linearized ETH_HLEN bytes of the inner Ethernet frame.
 - Link to v2: https://lore.kernel.org/netdev/20260916100155.1398403-1-edumazet@google.com/

v2:
 - Address Sashiko review feedback on v1.
 - Link to v1: https://lore.kernel.org/netdev/CANn89iJwwSMPXTjNYpXUskr1E+T8-EnkvO4LFSZAknqVL5+1gw@mail.gmail.com/

Assisted-by: LLM

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: do not read inner frame as erspan metadata in collect_md mode

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

-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net v3 1/5] ip_tunnel: do not clear the active encap before validating the new one

[Eric Dumazet]

ip_tunnel_encap_setup() and ip6_tnl_encap_setup() zero out t->encap with
memset() before calling ip_encap_hlen() / ip6_encap_hlen() to validate
the requested encapsulation type and module availability.

When a changelink request supplies an invalid encapsulation type or an
encapsulation whose module is not loaded, ip[6]_encap_hlen() returns
-EINVAL after t->encap has already been cleared to TUNNEL_ENCAP_NONE,
while t->encap_hlen and t->hlen remain at their previous values. A
rejected netlink request thus permanently disables FOU/GUE on a working
tunnel while keeping its reduced MTU and extra headroom.

The memset() is redundant: struct ip_tunnel_encap has exactly four
fields, and all of them are assigned unconditionally once the length
check has passed. A tunnel being created starts from the zeroed private
area of alloc_netdev(). Simply remove it, so that a failed
ip_tunnel_encap_setup() or ip6_tnl_encap_setup() leaves the active
encapsulation untouched. (Callers still invoke ip[6]_tunnel_encap_setup()
before validating the rest of a changelink request.)

net-next already does this for the IPv4 side 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.

Fixes: 56328486539d ("net: Changes to ip_tunnel to support foo-over-udp encapsulation")
Fixes: 058214a4d1df ("ip6_tun: Add infrastructure for doing encapsulation")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/ip_tunnel.c  | 2 --
 net/ipv6/ip6_tunnel.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index e6bcf01411d0bcd12cc9a88e449d9283c4a83c64..2a313b18134e2f0bafd52d59d8e173fa1a084f03 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -491,8 +491,6 @@ int ip_tunnel_encap_setup(struct ip_tunnel *t,
 {
 	int hlen;
 
-	memset(&t->encap, 0, sizeof(t->encap));
-
 	hlen = ip_encap_hlen(ipencap);
 	if (hlen < 0)
 		return hlen;
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index d5ff50a2ac01760acd1cb176c952f7113f733288..c918c2b0ad81b0161a2a98e4bd861436497c551c 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1818,8 +1818,6 @@ int ip6_tnl_encap_setup(struct ip6_tnl *t,
 {
 	int hlen;
 
-	memset(&t->encap, 0, sizeof(t->encap));
-
 	hlen = ip6_encap_hlen(ipencap);
 	if (hlen < 0)
 		return hlen;
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net v3 2/5] ip_gre: validate netlink attributes before changing the tunnel

[Eric Dumazet]

ipgre_netlink_parms() and erspan_netlink_parms() mutate the live device
and tunnel before all netlink attributes have been validated:

1. ipgre_netlink_parms() sets t->collect_md = true and changes dev->type
   from ARPHRD_IPGRE to ARPHRD_NONE before validating IFLA_GRE_IGNORE_DF
   or running ip_tunnel_changelink(). Moreover, unlike ipip_changelink()
   and ip6_tnl_changelink(), it accepts IFLA_GRE_COLLECT_METADATA during
   changelink even though ip_tunnel_changelink() never registers the
   tunnel as itn->collect_md_tun; a subsequent ip_tunnel_del() then
   clears itn->collect_md_tun and blackholes the netns metadata tunnel.

2. erspan_netlink_parms() writes t->erspan_ver, t->index, t->dir and
   t->hwid directly to the tunnel before validating the remaining
   attributes or running ip_tunnel_changelink(), leaving the live tunnel
   with a new ERSPAN version paired with the old flags and parameters
   when a later check fails.

Reject enabling IFLA_GRE_COLLECT_METADATA on an existing tunnel during
changelink, stage the GRE and ERSPAN attributes in a local struct
ip_gre_parm, validate the netlink parameters before setting up the
encapsulation, and commit them to the tunnel only after
ip_tunnel_changelink() has succeeded.

Fixes: e271c7b4420d ("gre: do not keep the GRE header around in collect medata mode")
Fixes: 84e54fe0a5ea ("gre: introduce native tunnel support for ERSPAN")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/ip_gre.c | 115 +++++++++++++++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 38 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 82309efd417e0f1f6554e7028be8e05d769e932d..df4d2f1f1d60c7f3755e4f554f04a06480512909 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1183,15 +1183,41 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
 	return 0;
 }
 
+struct ip_gre_parm {
+	__u32	fwmark;
+	u32	index;
+	u16	hwid;
+	u8	erspan_ver;
+	u8	dir;
+	bool	ignore_df;
+};
+
+static void ipgre_commit_parms(struct ip_tunnel *t,
+			       const struct ip_gre_parm *gparms)
+{
+	t->ignore_df = gparms->ignore_df;
+	t->erspan_ver = gparms->erspan_ver;
+	t->index = gparms->index;
+	t->hwid = gparms->hwid;
+	t->dir = gparms->dir;
+}
+
 static int ipgre_netlink_parms(struct net_device *dev,
 				struct nlattr *data[],
 				struct nlattr *tb[],
 				struct ip_tunnel_parm_kern *parms,
-				__u32 *fwmark)
+				struct ip_gre_parm *gparms,
+				bool newlink)
 {
 	struct ip_tunnel *t = netdev_priv(dev);
 
 	memset(parms, 0, sizeof(*parms));
+	gparms->fwmark = newlink ? 0 : t->fwmark;
+	gparms->ignore_df = t->ignore_df;
+	gparms->erspan_ver = t->erspan_ver;
+	gparms->index = t->index;
+	gparms->hwid = t->hwid;
+	gparms->dir = t->dir;
 
 	parms->iph.protocol = IPPROTO_GRE;
 
@@ -1234,20 +1260,24 @@ static int ipgre_netlink_parms(struct net_device *dev,
 	}
 
 	if (data[IFLA_GRE_COLLECT_METADATA]) {
-		t->collect_md = true;
-		if (dev->type == ARPHRD_IPGRE)
-			dev->type = ARPHRD_NONE;
+		if (!t->collect_md) {
+			if (!newlink)
+				return -EINVAL;
+			t->collect_md = true;
+			if (dev->type == ARPHRD_IPGRE)
+				dev->type = ARPHRD_NONE;
+		}
 	}
 
 	if (data[IFLA_GRE_IGNORE_DF]) {
 		if (nla_get_u8(data[IFLA_GRE_IGNORE_DF])
 		  && (parms->iph.frag_off & htons(IP_DF)))
 			return -EINVAL;
-		t->ignore_df = !!nla_get_u8(data[IFLA_GRE_IGNORE_DF]);
+		gparms->ignore_df = !!nla_get_u8(data[IFLA_GRE_IGNORE_DF]);
 	}
 
 	if (data[IFLA_GRE_FWMARK])
-		*fwmark = nla_get_u32(data[IFLA_GRE_FWMARK]);
+		gparms->fwmark = nla_get_u32(data[IFLA_GRE_FWMARK]);
 
 	return 0;
 }
@@ -1256,39 +1286,39 @@ static int erspan_netlink_parms(struct net_device *dev,
 				struct nlattr *data[],
 				struct nlattr *tb[],
 				struct ip_tunnel_parm_kern *parms,
-				__u32 *fwmark)
+				struct ip_gre_parm *gparms,
+				bool newlink)
 {
-	struct ip_tunnel *t = netdev_priv(dev);
 	int err;
 
-	err = ipgre_netlink_parms(dev, data, tb, parms, fwmark);
+	err = ipgre_netlink_parms(dev, data, tb, parms, gparms, newlink);
 	if (err)
 		return err;
 	if (!data)
 		return 0;
 
 	if (data[IFLA_GRE_ERSPAN_VER]) {
-		t->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
+		gparms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
 
-		if (t->erspan_ver > 2)
+		if (gparms->erspan_ver > 2)
 			return -EINVAL;
 	}
 
-	if (t->erspan_ver == 1) {
+	if (gparms->erspan_ver == 1) {
 		if (data[IFLA_GRE_ERSPAN_INDEX]) {
-			t->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
-			if (t->index & ~INDEX_MASK)
+			gparms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
+			if (gparms->index & ~INDEX_MASK)
 				return -EINVAL;
 		}
-	} else if (t->erspan_ver == 2) {
+	} else if (gparms->erspan_ver == 2) {
 		if (data[IFLA_GRE_ERSPAN_DIR]) {
-			t->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
-			if (t->dir & ~(DIR_MASK >> DIR_OFFSET))
+			gparms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
+			if (gparms->dir & ~(DIR_MASK >> DIR_OFFSET))
 				return -EINVAL;
 		}
 		if (data[IFLA_GRE_ERSPAN_HWID]) {
-			t->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
-			if (t->hwid & ~(HWID_MASK >> HWID_OFFSET))
+			gparms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
+			if (gparms->hwid & ~(HWID_MASK >> HWID_OFFSET))
 				return -EINVAL;
 		}
 	}
@@ -1401,7 +1431,12 @@ ipgre_newlink_encap_setup(struct net_device *dev, struct nlattr *data[])
 
 	if (ipgre_netlink_encap_parms(data, &ipencap)) {
 		struct ip_tunnel *t = netdev_priv(dev);
-		int err = ip_tunnel_encap_setup(t, &ipencap);
+		int err;
+
+		if (t->collect_md && ipencap.type != TUNNEL_ENCAP_NONE)
+			return -EINVAL;
+
+		err = ip_tunnel_encap_setup(t, &ipencap);
 
 		if (err < 0)
 			return err;
@@ -1417,18 +1452,19 @@ static int ipgre_newlink(struct net_device *dev,
 	struct nlattr **data = params->data;
 	struct nlattr **tb = params->tb;
 	struct ip_tunnel_parm_kern p;
-	__u32 fwmark = 0;
+	struct ip_gre_parm gparms;
 	int err;
 
 	err = ipgre_newlink_encap_setup(dev, data);
 	if (err)
 		return err;
 
-	err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
+	err = ipgre_netlink_parms(dev, data, tb, &p, &gparms, true);
 	if (err < 0)
 		return err;
+	ipgre_commit_parms(netdev_priv(dev), &gparms);
 	return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, &p,
-				 fwmark);
+				 gparms.fwmark);
 }
 
 static int erspan_newlink(struct net_device *dev,
@@ -1438,18 +1474,19 @@ static int erspan_newlink(struct net_device *dev,
 	struct nlattr **data = params->data;
 	struct nlattr **tb = params->tb;
 	struct ip_tunnel_parm_kern p;
-	__u32 fwmark = 0;
+	struct ip_gre_parm gparms;
 	int err;
 
 	err = ipgre_newlink_encap_setup(dev, data);
 	if (err)
 		return err;
 
-	err = erspan_netlink_parms(dev, data, tb, &p, &fwmark);
+	err = erspan_netlink_parms(dev, data, tb, &p, &gparms, true);
 	if (err)
 		return err;
+	ipgre_commit_parms(netdev_priv(dev), &gparms);
 	return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, &p,
-				 fwmark);
+				 gparms.fwmark);
 }
 
 static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
@@ -1458,24 +1495,25 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
 {
 	struct ip_tunnel *t = netdev_priv(dev);
 	struct ip_tunnel_parm_kern p;
-	__u32 fwmark = t->fwmark;
+	struct ip_gre_parm gparms;
 	int err;
 
 	if (!rtnl_dev_link_net_capable(dev, t->net))
 		return -EPERM;
 
-	err = ipgre_newlink_encap_setup(dev, data);
-	if (err)
+	err = ipgre_netlink_parms(dev, data, tb, &p, &gparms, false);
+	if (err < 0)
 		return err;
 
-	err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
-	if (err < 0)
+	err = ipgre_newlink_encap_setup(dev, data);
+	if (err)
 		return err;
 
-	err = ip_tunnel_changelink(dev, tb, &p, fwmark);
+	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
 	if (err < 0)
 		return err;
 
+	ipgre_commit_parms(t, &gparms);
 	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
 	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);
 
@@ -1490,24 +1528,25 @@ static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],
 {
 	struct ip_tunnel *t = netdev_priv(dev);
 	struct ip_tunnel_parm_kern p;
-	__u32 fwmark = t->fwmark;
+	struct ip_gre_parm gparms;
 	int err;
 
 	if (!rtnl_dev_link_net_capable(dev, t->net))
 		return -EPERM;
 
-	err = ipgre_newlink_encap_setup(dev, data);
-	if (err)
+	err = erspan_netlink_parms(dev, data, tb, &p, &gparms, false);
+	if (err < 0)
 		return err;
 
-	err = erspan_netlink_parms(dev, data, tb, &p, &fwmark);
-	if (err < 0)
+	err = ipgre_newlink_encap_setup(dev, data);
+	if (err)
 		return err;
 
-	err = ip_tunnel_changelink(dev, tb, &p, fwmark);
+	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
 	if (err < 0)
 		return err;
 
+	ipgre_commit_parms(t, &gparms);
 	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
 	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);
 
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net v3 3/5] ip_gre: compute tunnel lengths absolutely instead of by delta

[Eric Dumazet]

ipgre_link_update() shifts tunnel->hlen, dev->hard_header_len,
dev->needed_headroom and dev->mtu by the difference in tunnel->tun_hlen.
That misses any change to tunnel->encap_hlen, which ipgre_changelink() can
change through ipgre_newlink_encap_setup(). Such a request leaves @len
at zero: adding "encap fou" to an existing gre device keeps the MTU of a
bare tunnel, while creating it with "encap fou" from the start gets the
smaller MTU from ip_tunnel_bind_dev().

A difference is the wrong tool anyway: ip_tunnel_bind_dev() already
assigns dev->needed_headroom from tunnel->hlen, so changing the link and
the encapsulation at once is accounted twice, and ip_tunnel_encap_setup()
publishes a new tunnel->hlen before ip_tunnel_changelink() runs, making the
next difference bogus.

Recompute tunnel->hlen from tun_hlen and encap_hlen, as
__gre_tunnel_init() does, and add ip_tunnel_refresh_lengths() so that
ip_tunnel_bind_dev() is the only writer of dev->needed_headroom and
dev->mtu. ipgre_changelink() must then refresh on its
ip_tunnel_changelink() error path too, since the new encapsulation is
published by then.

dev->hard_header_len is recomputed the same way, but it stays in
ipgre_link_update(): for the ARPHRD_IPGRE devices installing
ipgre_header_ops it is the outer IP + GRE header, which
ip_tunnel_bind_dev() does not know about. Check specifically for
ipgre_header_ops: gretap devices also have header_ops (eth_header_ops),
where hard_header_len is the 14-byte inner Ethernet header that
ip_tunnel_bind_dev() subtracts from the MTU, and commit fdafed459998
("ip_gre: set dev->hard_header_len and dev->needed_headroom properly")
wrongly adjusted hard_header_len instead of needed_headroom for them.

@old_hlen survives only as a predicate telling whether the MTU became
stale, never as a difference, so it can not make the lengths drift. When
ip_tunnel_update() recomputes dev->mtu after a link or fwmark change using
the intermediate t->hlen published by ip_tunnel_encap_setup(), update
@old_hlen to that intermediate value so ipgre_link_update() still refreshes
dev->mtu if tun_hlen moves.

There is no memory safety issue: ipgre_xmit() cows dev->needed_headroom
at tunnel entry, which always reserves sizeof(struct iphdr) + lower
device headroom (>= 34 bytes after the GRE header), enough for
ip_tunnel_encap() to push the FOU/GUE header before ip_tunnel_xmit()
cows again for the outer IP header.

Fixes: dd9d598c6657 ("ip_gre: add the support for i/o_flags update via netlink")
Fixes: fdafed459998 ("ip_gre: set dev->hard_header_len and dev->needed_headroom properly")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ip_tunnels.h |  1 +
 net/ipv4/ip_gre.c        | 63 ++++++++++++++++++++++++++++++----------
 net/ipv4/ip_tunnel.c     | 17 +++++++++++
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 7c9aadfe8fe396da10a47e93499a97141ac04f4c..fd0396aa5039538a0391a352ace6d15f554df874 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -429,6 +429,7 @@ int ip_tunnel_newlink(struct net *net, struct net_device *dev,
 		      struct nlattr *tb[], struct ip_tunnel_parm_kern *p,
 		      __u32 fwmark);
 void ip_tunnel_setup(struct net_device *dev, unsigned int net_id);
+void ip_tunnel_refresh_lengths(struct net_device *dev, bool set_mtu);
 
 bool ip_tunnel_netlink_encap_parms(struct nlattr *data[],
 				   struct ip_tunnel_encap *encap);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index df4d2f1f1d60c7f3755e4f554f04a06480512909..27b3b4c584b1e1b4f1c9c9f42b5585b28101ece0 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -789,23 +789,36 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
-static void ipgre_link_update(struct net_device *dev, bool set_mtu)
+/* tunnel->hlen depends on tunnel->parms.o_flags and on tunnel->encap_hlen,
+ * both of which ipgre_changelink() can change. Recompute it the way
+ * __gre_tunnel_init() does, then let ip_tunnel_bind_dev() derive the device
+ * lengths from it.
+ *
+ * @old_hlen is only used to tell whether the MTU became stale, never as a
+ * difference to apply, so it can not make the lengths drift. It must be
+ * sampled before ip_tunnel_encap_setup(), which already publishes the new
+ * tunnel->hlen for us.
+ */
+static void ipgre_link_update(struct net_device *dev, bool set_mtu,
+			      int old_hlen)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
-	int len;
 
-	len = tunnel->tun_hlen;
 	tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
-	len = tunnel->tun_hlen - len;
-	tunnel->hlen = tunnel->hlen + len;
+	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen;
 
-	if (dev->header_ops)
-		dev->hard_header_len += len;
-	else
-		dev->needed_headroom += len;
+	/* For the ARPHRD_IPGRE devices installing ipgre_header_ops,
+	 * dev->hard_header_len is the outer IP + GRE header, as set by
+	 * ipgre_tunnel_init(). ip_tunnel_bind_dev() does not maintain it:
+	 * it only subtracts it from the MTU, and only for ARPHRD_ETHER.
+	 */
+	if (dev->header_ops == &ipgre_header_ops)
+		dev->hard_header_len = tunnel->hlen + sizeof(struct iphdr);
 
-	if (set_mtu)
-		WRITE_ONCE(dev->mtu, max_t(int, dev->mtu - len, 68));
+	/* Only reset a MTU that the header length just invalidated, so that
+	 * a MTU configured by the user survives an unrelated change.
+	 */
+	ip_tunnel_refresh_lengths(dev, set_mtu && tunnel->hlen != old_hlen);
 
 	if (test_bit(IP_TUNNEL_SEQ_BIT, tunnel->parms.o_flags) ||
 	    (test_bit(IP_TUNNEL_CSUM_BIT, tunnel->parms.o_flags) &&
@@ -853,7 +866,7 @@ static int ipgre_tunnel_ctl(struct net_device *dev,
 		ip_tunnel_flags_copy(t->parms.o_flags, p->o_flags);
 
 		if (strcmp(dev->rtnl_link_ops->kind, "erspan"))
-			ipgre_link_update(dev, true);
+			ipgre_link_update(dev, true, t->hlen);
 	}
 
 	i_flags = gre_tnl_flags_to_gre_flags(p->i_flags);
@@ -1496,6 +1509,8 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct ip_tunnel *t = netdev_priv(dev);
 	struct ip_tunnel_parm_kern p;
 	struct ip_gre_parm gparms;
+	int old_hlen = t->hlen;
+	bool link_changed;
 	int err;
 
 	if (!rtnl_dev_link_net_capable(dev, t->net))
@@ -1509,17 +1524,35 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (err)
 		return err;
 
+	link_changed = t->parms.link != p.link || t->fwmark != gparms.fwmark;
+
 	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
 	if (err < 0)
-		return err;
+		goto link_update;
+
+	/* When the link or fwmark changed, ip_tunnel_update() has just
+	 * recomputed dev->mtu from the intermediate t->hlen published by
+	 * ip_tunnel_encap_setup(). Record that as the length dev->mtu now
+	 * reflects so ipgre_link_update() refreshes it if tun_hlen moves.
+	 */
+	if (link_changed)
+		old_hlen = t->hlen;
 
 	ipgre_commit_parms(t, &gparms);
 	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
 	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);
 
-	ipgre_link_update(dev, !tb[IFLA_MTU]);
+link_update:
+	/* ipgre_newlink_encap_setup() has published a new encapsulation even
+	 * if ip_tunnel_changelink() failed, so the lengths must be refreshed
+	 * on that error path as well.
+	 *
+	 * IFLA_MTU only defers the MTU to do_setlink(), which rtnl_changelink()
+	 * does not reach if we return an error, so it must not hold it back.
+	 */
+	ipgre_link_update(dev, err || !tb[IFLA_MTU], old_hlen);
 
-	return 0;
+	return err;
 }
 
 static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 2a313b18134e2f0bafd52d59d8e173fa1a084f03..dd1b2f719f21670b02bfc1cafa62689b4a28a580 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -326,6 +326,23 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
 	return mtu;
 }
 
+/* Recompute dev->needed_headroom and dev->mtu after tunnel->hlen changed.
+ *
+ * Both are derived from tunnel->hlen, so they must be recomputed from it
+ * rather than adjusted by the difference: ip_tunnel_bind_dev() is also
+ * called from ip_tunnel_create(), ip_tunnel_newlink(), ip_tunnel_init_net()
+ * and ip_tunnel_update(), and a caller adding its own delta on top would
+ * double count it.
+ */
+void ip_tunnel_refresh_lengths(struct net_device *dev, bool set_mtu)
+{
+	int mtu = ip_tunnel_bind_dev(dev);
+
+	if (set_mtu)
+		WRITE_ONCE(dev->mtu, mtu);
+}
+EXPORT_SYMBOL_GPL(ip_tunnel_refresh_lengths);
+
 static struct ip_tunnel *ip_tunnel_create(struct net *net,
 					  struct ip_tunnel_net *itn,
 					  struct ip_tunnel_parm_kern *parms)
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net v3 4/5] ip_gre: recompute erspan header lengths after a change

[Eric Dumazet]

erspan_tunnel_init() is the only place computing tunnel->tun_hlen and
tunnel->hlen, but erspan_changelink() can change both: tunnel->erspan_ver
selects a 4 or 8 byte GRE header and feeds erspan_hdr_len(), while
ip_tunnel_encap_setup() recomputes tunnel->hlen without the ERSPAN part.

dev->needed_headroom is not refreshed either, since ip_tunnel_update()
only rebinds when the link or the fwmark changes.

erspan_xmit() then pushes an ERSPAN header sized from the new
tunnel->erspan_ver, while __gre_xmit() lays the GRE header out from the
stale tunnel->tun_hlen. After a version 0 -> 2 change, tun_hlen is still
4 and gre_build_header() writes the sequence number at
greh + tun_hlen - 4, that is over greh->flags and greh->protocol. The
MTU keeps the value derived from the old header length.

Also reject non-zero IFLA_GRE_OFLAGS/IFLA_GRE_IFLAGS when erspan_ver is 0
or IFLA_GRE_COLLECT_METADATA is set, and ensure IP_TUNNEL_SEQ_BIT in
erspan_xmit() matches tunnel->erspan_ver.

There is no memory safety issue: dev->needed_headroom is at least
tunnel->hlen + sizeof(struct iphdr), and erspan_xmit() pushes at most
12 + 8 bytes before ip_tunnel_xmit() takes over and cows again.

Move the computation into erspan_set_hlen() and add
erspan_link_update(), refreshing the lengths as the previous patch does
for plain GRE. Commit gparms alongside i_flags/o_flags after
ip_tunnel_changelink() succeeds, and run erspan_link_update() at the end
of erspan_changelink(), including on the ip_tunnel_changelink() error
path, as ipgre_changelink() does.

Fixes: f551c91de262 ("net: erspan: introduce erspan v2 for ip_gre")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/ip_gre.c | 81 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 64 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 27b3b4c584b1e1b4f1c9c9f42b5585b28101ece0..7385d66a94bf49c84bc674ded51ed3f9f5352e28 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -733,7 +733,6 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
 	/* Push ERSPAN header */
 	if (tunnel->erspan_ver == 0) {
 		proto = htons(ETH_P_ERSPAN);
-		__clear_bit(IP_TUNNEL_SEQ_BIT, flags);
 	} else if (tunnel->erspan_ver == 1) {
 		erspan_build_header(skb, ntohl(tunnel->parms.o_key),
 				    tunnel->index,
@@ -748,6 +747,7 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
 		goto free_skb;
 	}
 
+	__assign_bit(IP_TUNNEL_SEQ_BIT, flags, tunnel->erspan_ver != 0);
 	__clear_bit(IP_TUNNEL_KEY_BIT, flags);
 	__gre_xmit(skb, dev, &tunnel->parms.iph, proto, flags);
 	return NETDEV_TX_OK;
@@ -1169,17 +1169,18 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
 	if (ret)
 		return ret;
 
-	if (data[IFLA_GRE_ERSPAN_VER] &&
-	    nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0)
-		return 0;
-
-	/* ERSPAN type II/III should only have GRE sequence and key flag */
 	if (data[IFLA_GRE_OFLAGS])
 		flags |= nla_get_be16(data[IFLA_GRE_OFLAGS]);
 	if (data[IFLA_GRE_IFLAGS])
 		flags |= nla_get_be16(data[IFLA_GRE_IFLAGS]);
-	if (!data[IFLA_GRE_COLLECT_METADATA] &&
-	    flags != (GRE_SEQ | GRE_KEY))
+
+	if ((data[IFLA_GRE_ERSPAN_VER] &&
+	     nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0) ||
+	    data[IFLA_GRE_COLLECT_METADATA])
+		return flags ? -EINVAL : 0;
+
+	/* ERSPAN type II/III should only have GRE sequence and key flag */
+	if (flags != (GRE_SEQ | GRE_KEY))
 		return -EINVAL;
 
 	/* ERSPAN Session ID only has 10-bit. Since we reuse
@@ -1317,7 +1318,11 @@ static int erspan_netlink_parms(struct net_device *dev,
 			return -EINVAL;
 	}
 
-	if (gparms->erspan_ver == 1) {
+	if (gparms->erspan_ver == 0) {
+		if (!ip_tunnel_flags_empty(parms->i_flags) ||
+		    !ip_tunnel_flags_empty(parms->o_flags))
+			return -EINVAL;
+	} else if (gparms->erspan_ver == 1) {
 		if (data[IFLA_GRE_ERSPAN_INDEX]) {
 			gparms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
 			if (gparms->index & ~INDEX_MASK)
@@ -1394,18 +1399,43 @@ static const struct net_device_ops gre_tap_netdev_ops = {
 	.ndo_fill_metadata_dst	= gre_fill_metadata_dst,
 };
 
+static void erspan_set_hlen(struct ip_tunnel *tunnel)
+{
+	/* Version 0 uses a 4-byte GRE header, other versions use 8 bytes. */
+	tunnel->tun_hlen = tunnel->erspan_ver == 0 ? 4 : 8;
+
+	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen +
+		       erspan_hdr_len(tunnel->erspan_ver);
+}
+
+/* Both tunnel->erspan_ver and tunnel->encap_hlen can be changed from
+ * erspan_changelink(), and both feed tunnel->hlen. Recompute it, then let
+ * ip_tunnel_bind_dev() derive the device lengths from it.
+ *
+ * As in ipgre_link_update(), @old_hlen only tells whether the MTU became
+ * stale and must be sampled before ip_tunnel_encap_setup(), which
+ * recomputes tunnel->hlen without the ERSPAN part.
+ */
+static void erspan_link_update(struct net_device *dev, bool set_mtu,
+			       int old_hlen)
+{
+	struct ip_tunnel *tunnel = netdev_priv(dev);
+
+	erspan_set_hlen(tunnel);
+
+	/* Only reset a MTU that the header length just invalidated, so that
+	 * a MTU configured by the user survives an unrelated change.
+	 */
+	ip_tunnel_refresh_lengths(dev, set_mtu && tunnel->hlen != old_hlen);
+}
+
 static int erspan_tunnel_init(struct net_device *dev)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 
-	if (tunnel->erspan_ver == 0)
-		tunnel->tun_hlen = 4; /* 4-byte GRE hdr. */
-	else
-		tunnel->tun_hlen = 8; /* 8-byte GRE hdr. */
+	erspan_set_hlen(tunnel);
 
 	tunnel->parms.iph.protocol = IPPROTO_GRE;
-	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen +
-		       erspan_hdr_len(tunnel->erspan_ver);
 
 	dev->features		|= GRE_FEATURES;
 	dev->hw_features	|= GRE_FEATURES;
@@ -1562,6 +1592,8 @@ static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct ip_tunnel *t = netdev_priv(dev);
 	struct ip_tunnel_parm_kern p;
 	struct ip_gre_parm gparms;
+	int old_hlen = t->hlen;
+	bool link_changed;
 	int err;
 
 	if (!rtnl_dev_link_net_capable(dev, t->net))
@@ -1575,15 +1607,30 @@ static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (err)
 		return err;
 
+	link_changed = t->parms.link != p.link || t->fwmark != gparms.fwmark;
+
 	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
 	if (err < 0)
-		return err;
+		goto link_update;
+
+	if (link_changed)
+		old_hlen = t->hlen;
 
 	ipgre_commit_parms(t, &gparms);
 	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
 	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);
 
-	return 0;
+link_update:
+	/* ipgre_newlink_encap_setup() has published a new encapsulation
+	 * without the ERSPAN header length even if ip_tunnel_changelink()
+	 * failed, so the lengths must be refreshed on that error path too.
+	 *
+	 * As in ipgre_changelink(), IFLA_MTU must not hold the MTU back if we
+	 * return an error, because do_setlink() will not apply it then.
+	 */
+	erspan_link_update(dev, err || !tb[IFLA_MTU], old_hlen);
+
+	return err;
 }
 
 static size_t ipgre_get_size(const struct net_device *dev)
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH net v3 5/5] gre: do not read inner frame as erspan metadata in collect_md mode

[Eric Dumazet]

erspan_rcv() and ip6erspan_rcv() copy the ERSPAN metadata out of the
packet for collect_md tunnels:

        pkt_md = (struct erspan_metadata *)(gh + gre_hdr_len +
                                            sizeof(*ershdr));
        ...
        memcpy(md2, pkt_md, ver == 1 ? ERSPAN_V1_MDSIZE :
                                       ERSPAN_V2_MDSIZE);

Both read 8 bytes at 12 bytes from the start of the GRE header, assuming
an ERSPAN version 1 or 2 header was pulled. Because
__iptunnel_pull_header() with ETH_P_TEB linearizes an extra ETH_HLEN (14)
bytes beyond @len, this does not read past skb->tail, but when
erspan_hdr_len(ver) is 0 it copies bytes from the inner Ethernet frame
into md->u.md2. Two ways to get there:

- An ERSPAN type I packet has a 4-byte ETH_P_ERSPAN GRE header and no
  ERSPAN header at all, so erspan_rcv() sets ver = 0 and only pulls the
  GRE header. It still falls back to a collect_md tunnel through
  itn->collect_md_tun, and there the ternary above picks ERSPAN_V2_MDSIZE
  and reads 8 bytes of the inner Ethernet header as ERSPAN v2 metadata.
  Also check tpi->proto == htons(ETH_P_ERSPAN) in is_erspan_type1() to
  match gre_parse_header() so a 4-byte ETH_P_ERSPAN2 frame is not treated
  as Type I.

- A packet with an 8-byte GRE header (or IPv6 ERSPAN) and ershdr->ver == 0
  is malformed, yet neither erspan_rcv() nor ip6erspan_rcv() validates the
  version before using it, so erspan_hdr_len(0) pulls 0 bytes (leaving the
  4-byte ERSPAN base header inside the inner frame) and copies inner frame
  bytes into md->u.md2. A version above 2 also stores a version that the
  transmit side (erspan_fb_xmit(), ip6erspan_tunnel_xmit()) rejects.

Reject a base header whose version is neither 1 nor 2, and skip the
metadata extraction for type I, which has none: ip_tun_rx_dst() hands
out a zeroed option area, so md->version = 0 alone describes it.

Fixes: f989d546a2d5 ("erspan: Add type I version 0 support.")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/ip_gre.c  | 43 +++++++++++++++++++++++++++----------------
 net/ipv6/ip6_gre.c |  2 ++
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 7385d66a94bf49c84bc674ded51ed3f9f5352e28..a00df7bda0012b03aed50fd569cfd0611a67ce9f 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -255,13 +255,13 @@ static void gre_err(struct sk_buff *skb, u32 info)
 	ipgre_err(skb, info, &tpi);
 }
 
-static bool is_erspan_type1(int gre_hdr_len)
+static bool is_erspan_type1(int gre_hdr_len, __be16 proto)
 {
 	/* Both ERSPAN type I (version 0) and type II (version 1) use
 	 * protocol 0x88BE, but the type I has only 4-byte GRE header,
 	 * while type II has 8-byte.
 	 */
-	return gre_hdr_len == 4;
+	return proto == htons(ETH_P_ERSPAN) && gre_hdr_len == 4;
 }
 
 static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
@@ -274,7 +274,6 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 	struct ip_tunnel_net *itn;
 	struct ip_tunnel *tunnel;
 	const struct iphdr *iph;
-	struct erspan_md2 *md2;
 	int ver;
 	int len;
 
@@ -282,7 +281,7 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 
 	itn = net_generic(net, erspan_net_id);
 	iph = ip_hdr(skb);
-	if (is_erspan_type1(gre_hdr_len)) {
+	if (is_erspan_type1(gre_hdr_len, tpi->proto)) {
 		ver = 0;
 		__set_bit(IP_TUNNEL_NO_KEY_BIT, flags);
 		tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex, flags,
@@ -294,6 +293,9 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 
 		ershdr = (struct erspan_base_hdr *)(skb->data + gre_hdr_len);
 		ver = ershdr->ver;
+		if (unlikely(ver != 1 && ver != 2))
+			return PACKET_REJECT;
+
 		iph = ip_hdr(skb);
 		__set_bit(IP_TUNNEL_KEY_BIT, flags);
 		tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex, flags,
@@ -301,7 +303,7 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 	}
 
 	if (tunnel) {
-		if (is_erspan_type1(gre_hdr_len))
+		if (is_erspan_type1(gre_hdr_len, tpi->proto))
 			len = gre_hdr_len;
 		else
 			len = gre_hdr_len + erspan_hdr_len(ver);
@@ -318,6 +320,7 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 		if (tunnel->collect_md) {
 			struct erspan_metadata *pkt_md, *md;
 			struct ip_tunnel_info *info;
+			struct erspan_md2 *md2;
 			unsigned char *gh;
 			__be64 tun_id;
 
@@ -334,19 +337,27 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 			info = &tun_dst->u.tun_info;
 			info->options_len = sizeof(*md);
 
-			/* skb can be uncloned in __iptunnel_pull_header, so
-			 * old pkt_md is no longer valid and we need to reset
-			 * it
-			 */
-			gh = skb_network_header(skb) +
-			     skb_network_header_len(skb);
-			pkt_md = (struct erspan_metadata *)(gh + gre_hdr_len +
-							    sizeof(*ershdr));
 			md = ip_tunnel_info_opts(&tun_dst->u.tun_info);
 			md->version = ver;
-			md2 = &md->u.md2;
-			memcpy(md2, pkt_md, ver == 1 ? ERSPAN_V1_MDSIZE :
-						       ERSPAN_V2_MDSIZE);
+
+			/* Type I has no ERSPAN header, thus no metadata to
+			 * extract: pkt_md would point into the inner Ethernet
+			 * frame just pulled by __iptunnel_pull_header().
+			 * ip_tun_rx_dst() zeroed @md for us.
+			 */
+			if (!is_erspan_type1(gre_hdr_len, tpi->proto)) {
+				/* skb can be uncloned in __iptunnel_pull_header, so
+				 * old pkt_md is no longer valid and we need to reset
+				 * it
+				 */
+				gh = skb_network_header(skb) +
+				     skb_network_header_len(skb);
+				pkt_md = (struct erspan_metadata *)(gh + gre_hdr_len +
+								    sizeof(*ershdr));
+				md2 = &md->u.md2;
+				memcpy(md2, pkt_md, ver == 1 ? ERSPAN_V1_MDSIZE :
+							       ERSPAN_V2_MDSIZE);
+			}
 
 			__set_bit(IP_TUNNEL_ERSPAN_OPT_BIT,
 				  info->key.tun_flags);
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index e61cb10b50dc96295c14aa6da4c9c0fe47151335..774975955747ec4d822b66b8aaaee824c8d331df 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -503,6 +503,8 @@ static int ip6erspan_rcv(struct sk_buff *skb,
 	ipv6h = ipv6_hdr(skb);
 	ershdr = (struct erspan_base_hdr *)skb->data;
 	ver = ershdr->ver;
+	if (unlikely(ver != 1 && ver != 2))
+		return PACKET_REJECT;
 
 	tunnel = ip6gre_tunnel_lookup(skb->dev,
 				      &ipv6h->saddr, &ipv6h->daddr, tpi->key,
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH net v3 1/5] ip_tunnel: do not clear the active encap before validating the new one

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (1) · 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).

- [Medium] The patch only fixes the case where
  ip_tunnel_encap_setup()/ip6_tnl_encap_setup() itself fails.

Pre-existing issues:
- [Medium] ip_tunnel_encap_setup() and ip6_tnl_encap_setup() update
  t->encap.{type,sport,dport,flags}, t->encap_hlen and t->hlen one field…

--

ip_tunnel: do not clear the active encap before validating the new one

This patch removes the memset() of t->encap from ip_tunnel_encap_setup()
and ip6_tnl_encap_setup(). After this change, a request that
ip_encap_hlen() or ip6_encap_hlen() rejects no longer leaves the tunnel
set to TUNNEL_ENCAP_NONE with stale encap_hlen and hlen values. The patch
is tagged for stable.

> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index e6bcf01411d0b..2a313b18134e2 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -491,8 +491,6 @@ int ip_tunnel_encap_setup(struct ip_tunnel *t,
>  {
>  	int hlen;
>  
> -	memset(&t->encap, 0, sizeof(t->encap));
> -
>  	hlen = ip_encap_hlen(ipencap);
>  	if (hlen < 0)
>  		return hlen;

[Severity: Medium]
This covers the case where ip_tunnel_encap_setup() itself fails. Can the
symptom described in the commit message still happen when encap setup
succeeds and the caller rejects the request afterwards?

The commit message says:

  "A rejected netlink request thus permanently disables FOU/GUE on a
  working tunnel while keeping its reduced MTU and extra headroom."

Once ip_encap_hlen() succeeds, ip_tunnel_encap_setup() writes
t->encap.{type,sport,dport,flags}, t->encap_hlen and t->hlen into the
live tunnel. In ipip_changelink(), the rest of the request is only
validated after that:

net/ipv4/ipip.c:ipip_changelink() {
    ...
	if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
		int err = ip_tunnel_encap_setup(t, &ipencap);

		if (err < 0)
			return err;
	}

	ipip_netlink_parms(data, &p, &collect_md, &fwmark);
	if (collect_md)
		return -EINVAL;
    ...
}

Take a changelink on an existing FOU/GUE ipip tunnel that sends
IFLA_IPTUN_ENCAP_TYPE=TUNNEL_ENCAP_NONE together with
IFLA_IPTUN_COLLECT_METADATA:

- ip_encap_hlen() returns 0, so the encap is cleared and t->hlen shrinks.
- The collect_md check then returns -EINVAL.
- Userspace gets an error, but FOU/GUE is now off.
- dev->mtu keeps its reduced value, because ip_tunnel_update() never ran.

These other rejections also happen after the encap has been committed.
None of them roll it back or refresh dev->mtu or the headroom:

- ipip on the fallback device: "ip link change tunl0 encap fou ..."
  applies FOU to tunl0. ip_tunnel_changelink() then returns -EINVAL at
  "if (dev == itn->fb_tunnel_dev)".
- ipip_changelink() and ipip6_changelink(): the IFF_POINTOPOINT/daddr
  -EINVAL check, and the -EEXIST returns from ip_tunnel_changelink() and
  ipip6_tunnel_locate().
- ip6_tnl_changelink(): the "if (p.collect_md) return -EINVAL;" check and
  the -EEXIST from ip6_tnl_locate(). Both skip ip6_tnl_update().
- ip6gre_changelink_common(): the -EEXIST from ip6gre_tunnel_locate().

Later in this series, "ip_gre: validate netlink attributes before
changing the tunnel" fixes ip_gre. It moves ipgre_netlink_parms() first
and refreshes the lengths on the ip_tunnel_changelink() error path. At
the end of the series, ipip, sit, ip6_tunnel and ip6_gre are unchanged.

The commit message says callers still call ip[6]_tunnel_encap_setup()
before validating the rest of a changelink request. The patch is tagged
Fixes/Cc: stable for this symptom. Should these callers also validate the
whole request before they commit the new encap?

> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index d5ff50a2ac017..c918c2b0ad81b 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1818,8 +1818,6 @@ int ip6_tnl_encap_setup(struct ip6_tnl *t,
>  {
>  	int hlen;
>  
> -	memset(&t->encap, 0, sizeof(t->encap));
> -
>  	hlen = ip6_encap_hlen(ipencap);
>  	if (hlen < 0)
>  		return hlen;

[Severity: Medium]
This isn't a bug introduced by this patch, but it concerns the same
fields. ip_tunnel_encap_setup() and ip6_tnl_encap_setup() update
t->encap.{type,sport,dport,flags}, t->encap_hlen and t->hlen one at a
time with plain stores under RTNL. The transmit paths read those fields
with no lock and no snapshot.

In ip6_tnl_xmit(), encap_hlen is read on entry and later sizes
skb_cow_head():

net/ipv6/ip6_tunnel.c:ip6_tnl_xmit() {
    ...
	unsigned int psh_hlen = sizeof(struct ipv6hdr) + t->encap_hlen;
	unsigned int max_headroom = psh_hlen;
    ...
}

Much later, ip6_tnl_encap() reads t->encap.type again to pick the
callback, and it passes the mutable t->encap:

include/net/ip6_tunnel.h:ip6_tnl_encap() {
    ...
	ops = rcu_dereference(ip6tun_encaps[t->encap.type]);
	if (likely(ops && ops->build_header))
		ret = ops->build_header(skb, &t->encap, protocol, fl6);
    ...
}

Suppose a changelink switches from NONE to GUE between these two reads.
Could headroom be reserved with encap_hlen == 0 while a GUE header is
pushed? If the skb had only the minimum headroom, this might reach
skb_under_panic(), though it is not clear that such skbs reach this
path.

Could fou6_build_udp() also read a dport/flags pair from the next
configuration after the callback has already been chosen?

The IPv4 side has the same pattern. ip_tunnel_xmit() reads
tunnel->encap once for the header and again for the headroom:

net/ipv4/ip_tunnel.c:ip_tunnel_xmit() {
    ...
	if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0)
		goto tx_error;
    ...
	max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
			+ rt->dst.header_len + ip_encap_hlen(&tunnel->encap);
    ...
}

None of these accesses use READ_ONCE()/WRITE_ONCE(), so KCSAN would
likely report them. Removing the memset makes the window smaller, because
readers no longer see a temporary TUNNEL_ENCAP_NONE. According to the
commit message, the net-next commit 88b84cae6b94 adds WRITE_ONCE() only
on the IPv4 side. Should the readers, and the IPv6 writer, get matching
annotations or use a consistent snapshot of the encap?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923035217.179102-1-edumazet%40google.com

Re: [PATCH net v3 2/5] ip_gre: validate netlink attributes before changing the tunnel

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 3 (1)

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

- [Medium] ipgre_changelink() and erspan_changelink() still apply the new
  encapsulation to the live tunnel before ip_tunnel_changelink(), and…
- [Low] The commit message misstates how the COLLECT_METADATA changelink
  bug works.
- [Low] The patch adds a new check to ipgre_newlink_encap_setup() that
  the commit message does not mention: `if (t->collect_md &&…
- [Low] The patch moves ipgre_commit_parms() after
  ip_tunnel_changelink().

Pre-existing issues:
- [Medium] When erspan_changelink() succeeds, ipgre_commit_parms() writes
  the new t->erspan_ver, but tunnel->tun_hlen and tunnel->hlen are not…
- [Low] ipgre_commit_parms() writes t->erspan_ver, t->index, t->hwid,
  t->dir and t->ignore_df as separate plain stores under RTNL only.

--

ip_gre: validate netlink attributes before changing the tunnel

This commit stages the GRE and ERSPAN netlink attributes in a new local
struct ip_gre_parm. It commits them to the tunnel only after
ip_tunnel_changelink() succeeds. It also rejects enabling
IFLA_GRE_COLLECT_METADATA on an existing tunnel during changelink, and
moves encap setup after parameter validation in the changelink handlers.

> 1. ipgre_netlink_parms() sets t->collect_md = true and changes dev->type
>    from ARPHRD_IPGRE to ARPHRD_NONE before validating IFLA_GRE_IGNORE_DF
>    or running ip_tunnel_changelink(). Moreover, unlike ipip_changelink()
>    and ip6_tnl_changelink(), it accepts IFLA_GRE_COLLECT_METADATA during
>    changelink even though ip_tunnel_changelink() never registers the
>    tunnel as itn->collect_md_tun; a subsequent ip_tunnel_del() then
>    clears itn->collect_md_tun and blackholes the netns metadata tunnel.

[Severity: Low]
Is the "never registers" part accurate for the success path?

Before this patch, ipgre_netlink_parms() set t->collect_md = true before
ip_tunnel_changelink() ran. On success, ip_tunnel_changelink() calls
ip_tunnel_update(), which calls ip_tunnel_del() and then
ip_tunnel_add():

net/ipv4/ip_tunnel.c:ip_tunnel_add() {
    ...
	if (t->collect_md)
		rcu_assign_pointer(itn->collect_md_tun, t);
    ...
}

net/ipv4/ip_tunnel.c:ip_tunnel_del() {
	if (t->collect_md)
		rcu_assign_pointer(itn->collect_md_tun, NULL);
    ...
}

So a successful changelink evicts any existing metadata tunnel in the
netns and registers this tunnel in its place. Deleting either device
afterwards clears the slot.

The tunnel only ends up with collect_md set but unregistered on the
failure paths. These are the IFLA_GRE_IGNORE_DF -EINVAL, an encap setup
error, or ip_tunnel_changelink() failing before it reaches
ip_tunnel_update().

The fix itself looks correct. Since this is Cc: stable, could the
description also cover the success-path takeover of itn->collect_md_tun?

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 82309efd417e0..df4d2f1f1d60c 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1183,15 +1183,41 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
>  	return 0;
>  }
>  
> +struct ip_gre_parm {
> +	__u32	fwmark;
> +	u32	index;
> +	u16	hwid;
> +	u8	erspan_ver;
> +	u8	dir;
> +	bool	ignore_df;
> +};
> +
> +static void ipgre_commit_parms(struct ip_tunnel *t,
> +			       const struct ip_gre_parm *gparms)
> +{
> +	t->ignore_df = gparms->ignore_df;
> +	t->erspan_ver = gparms->erspan_ver;
> +	t->index = gparms->index;
> +	t->hwid = gparms->hwid;
> +	t->dir = gparms->dir;
> +}

[Severity: Low]
This isn't a bug introduced by this patch, but these are plain stores
made under RTNL only. The i_flags/o_flags copies follow as separate
steps. erspan_xmit() and __gre_xmit() read the same fields without
synchronization.

Can a transmit that races with changelink build a packet that mixes a
new erspan_ver with the old index or hwid, or with the old tun_hlen?

The old code had the same unsynchronized writes in
erspan_netlink_parms(), and this patch doesn't widen the window. At
worst, headers could be malformed for a short time during
reconfiguration, and KCSAN could report the plain concurrent accesses.

[ ... ]

> @@ -1401,7 +1431,12 @@ ipgre_newlink_encap_setup(struct net_device *dev, struct nlattr *data[])
>  
>  	if (ipgre_netlink_encap_parms(data, &ipencap)) {
>  		struct ip_tunnel *t = netdev_priv(dev);
> -		int err = ip_tunnel_encap_setup(t, &ipencap);
> +		int err;
> +
> +		if (t->collect_md && ipencap.type != TUNNEL_ENCAP_NONE)
> +			return -EINVAL;

[Severity: Low]
The commit message doesn't mention this check. Is it meant as a
separate behavior change?

In ipgre_newlink() and erspan_newlink(), ipgre_newlink_encap_setup()
runs before ipgre_netlink_parms() sets t->collect_md, so the check can't
fire when the device is created. For creation, ipgre_tunnel_validate()
already rejects COLLECT_METADATA combined with a non-NONE ENCAP_TYPE in
the same request.

The check only takes effect in ipgre_changelink() and
erspan_changelink(). There, ipgre_netlink_parms() now runs first and
t->collect_md is live device state.

Consider a changelink on an existing external GRE/GRETAP/ERSPAN device
that sets IFLA_GRE_ENCAP_TYPE to FOU or GUE and omits
IFLA_GRE_COLLECT_METADATA. That request used to be accepted and now
fails with -EINVAL.

The restriction may well be right, since ip_md_tunnel_xmit() uses
tun_info->encap and not t->encap. Could the commit message describe
this userspace-visible change and explain why it is needed, given the
stable tag?

> +
> +		err = ip_tunnel_encap_setup(t, &ipencap);
>  
>  		if (err < 0)
>  			return err;

[ ... ]

> @@ -1458,24 +1495,25 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
>  {
>  	struct ip_tunnel *t = netdev_priv(dev);
>  	struct ip_tunnel_parm_kern p;
> -	__u32 fwmark = t->fwmark;
> +	struct ip_gre_parm gparms;
>  	int err;
>  
>  	if (!rtnl_dev_link_net_capable(dev, t->net))
>  		return -EPERM;
>  
> -	err = ipgre_newlink_encap_setup(dev, data);
> -	if (err)
> +	err = ipgre_netlink_parms(dev, data, tb, &p, &gparms, false);
> +	if (err < 0)
>  		return err;
>  
> -	err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
> -	if (err < 0)
> +	err = ipgre_newlink_encap_setup(dev, data);
> +	if (err)
>  		return err;
>  
> -	err = ip_tunnel_changelink(dev, tb, &p, fwmark);
> +	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
>  	if (err < 0)
>  		return err;

[Severity: Medium]
ipgre_newlink_encap_setup() still applies the new encapsulation to the
live tunnel before ip_tunnel_changelink(), and that call can still fail.
Does this error return leave the device with the new encap and the old
parameters?

ipgre_newlink_encap_setup()->ip_tunnel_encap_setup() writes straight to
the tunnel:

net/ipv4/ip_tunnel.c:ip_tunnel_encap_setup() {
    ...
	t->encap.type = ipencap->type;
	t->encap.sport = ipencap->sport;
	t->encap.dport = ipencap->dport;
	t->encap.flags = ipencap->flags;

	t->encap_hlen = hlen;
	t->hlen = t->encap_hlen + t->tun_hlen;
    ...
}

After that, ip_tunnel_changelink() can still fail, for example here:

net/ipv4/ip_tunnel.c:ip_tunnel_changelink() {
    ...
	if (dev == itn->fb_tunnel_dev)
		return -EINVAL;

	t = ip_tunnel_find(itn, p, dev->type);

	if (t) {
		if (t->dev != dev)
			return -EEXIST;
    ...
}

It can also return -EINVAL on an IFF_POINTOPOINT/IFF_BROADCAST
mismatch.

Take "ip link set gre0 type gre encap fou encap-dport N" on the fallback
device, or pointing gre2 at gre1's remote while also adding "encap fou".
Both return an error, but the device now sends FOU/GUE-encapsulated
traffic with its old parms, fwmark, flags and gparms. This patch also
doesn't refresh the MTU or needed_headroom on this error path.

The commit message says the attributes are committed "only after
ip_tunnel_changelink() has succeeded", but the encap is still committed
before that point. erspan_changelink() uses the same ordering.

Later patches in the series ("ip_gre: compute tunnel lengths absolutely
instead of by delta" and "ip_gre: recompute erspan header lengths after
a change") only refresh the lengths on the error path. The partially
applied encap is therefore still there at the end of the series.

Could the encap be staged too, and applied only after
ip_tunnel_changelink() succeeds?

>  
> +	ipgre_commit_parms(t, &gparms);
>  	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
>  	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);

[Severity: Low]
ip_tunnel_changelink()->ip_tunnel_update() ends with
netdev_state_change(dev). That runs the NETDEV_CHANGE notifiers and
sends an RTM_NEWLINK built by ipgre_fill_info() or erspan_fill_info().

With ipgre_commit_parms() now after that call, does the notification
report the old t->ignore_df? For ERSPAN, does it also report the old
erspan_ver, index, dir and hwid?

Before this patch, those fields were written ahead of
ip_tunnel_changelink(), so the first notification had the new values.
rtnl_changelink() sets DO_SETLINK_NOTIFY, so do_setlink() sends a
second, correct notification afterwards. Listeners still see one
transient RTM_NEWLINK with stale values. The ordering is unchanged at
the end of the series.

> @@ -1490,24 +1528,25 @@ static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],

[ ... ]

> -	err = ip_tunnel_changelink(dev, tb, &p, fwmark);
> +	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
>  	if (err < 0)
>  		return err;
>  
> +	ipgre_commit_parms(t, &gparms);
>  	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
>  	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);

[Severity: Medium]
This is a pre-existing issue, but when erspan_changelink() succeeds with
a different IFLA_GRE_ERSPAN_VER, ipgre_commit_parms() updates
t->erspan_ver and does not recompute tunnel->tun_hlen or tunnel->hlen.
In this patch those are derived from erspan_ver only in
erspan_tunnel_init():

net/ipv4/ip_gre.c:erspan_tunnel_init() {
    ...
	if (tunnel->erspan_ver == 0)
		tunnel->tun_hlen = 4; /* 4-byte GRE hdr. */
	else
		tunnel->tun_hlen = 8; /* 8-byte GRE hdr. */
    ...
}

__gre_xmit() builds the GRE header from tunnel->tun_hlen. Can a v0 to
v1/v2 change then produce malformed headers, with the sequence number
overwriting the flags/protocol, plus the wrong MTU and headroom?

The old code also changed erspan_ver without recomputing these lengths.
A later patch in the series, "ip_gre: recompute erspan header lengths
after a change", fixes this by calling erspan_link_update() from
erspan_changelink().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923035217.179102-1-edumazet%40google.com

Re: [PATCH net v3 3/5] ip_gre: compute tunnel lengths absolutely instead of by delta

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 (1) · 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).

- [Medium] When the GRE header length changes, ipgre_link_update() now
  replaces the user-configured dev->mtu with the default computed by…
- [Medium] A changelink that publishes a new header length but carries an
  IFLA_MTU that do_setlink() later rejects leaves the tunnel with the…

Pre-existing issues:
- [Medium] Sibling changelink handlers publish a new encap_hlen through
  ip_tunnel_encap_setup() but never recompute dev->mtu or…

--

ip_gre: compute tunnel lengths absolutely instead of by delta

This commit changes ipgre_link_update() to recompute tunnel->hlen from
tun_hlen and encap_hlen instead of applying a delta. It adds
ip_tunnel_refresh_lengths() so that ip_tunnel_bind_dev() is the only
writer of dev->needed_headroom and dev->mtu. ipgre_changelink() now also
refreshes the lengths on the ip_tunnel_changelink() error path.

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index df4d2f1f1d60c..27b3b4c584b1e 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c

[ ... ]

> +static void ipgre_link_update(struct net_device *dev, bool set_mtu,
> +			      int old_hlen)
>  {
>  	struct ip_tunnel *tunnel = netdev_priv(dev);
> -	int len;
>  
> -	len = tunnel->tun_hlen;
>  	tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
> -	len = tunnel->tun_hlen - len;
> -	tunnel->hlen = tunnel->hlen + len;
> +	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen;

[ ... ]

> -	if (set_mtu)
> -		WRITE_ONCE(dev->mtu, max_t(int, dev->mtu - len, 68));
> +	/* Only reset a MTU that the header length just invalidated, so that
> +	 * a MTU configured by the user survives an unrelated change.
> +	 */
> +	ip_tunnel_refresh_lengths(dev, set_mtu && tunnel->hlen != old_hlen);

[Severity: Medium]
Can this overwrite an MTU that the user configured? Before this patch, a
change to tun_hlen reduced the current dev->mtu by the delta:

    WRITE_ONCE(dev->mtu, max_t(int, dev->mtu - len, 68));

Now, when hlen differs from old_hlen, ip_tunnel_refresh_lengths() writes
the value that ip_tunnel_bind_dev() computes from the underlay. The
current dev->mtu is not taken into account.

The ioctl path always passes set_mtu = true:

ipgre_tunnel_ctl()
    ipgre_link_update(dev, true, t->hlen)
        ip_tunnel_refresh_lengths(dev, true)
            WRITE_ONCE(dev->mtu, ip_tunnel_bind_dev(dev))

ipgre_changelink() does the same whenever IFLA_MTU is absent.

Take gre1 over a 1500 byte eth0 with MTU 1300 set, and run
"ip link set gre1 type gre okey 1" or "ip tunnel change gre1 key 1". The
MTU becomes 1472 instead of 1296, which is larger than the value the
user chose.

If ip_tunnel_bind_dev() finds no underlay device (daddr == 0 with no
link, or a failed route lookup), the MTU falls back to
ETH_DATA_LEN - t_hlen. A jumbo MTU of 8976 would then drop to 1472.

The comment says the header length change invalidated the MTU. A user
MTU that is still below the new maximum still looks valid, though. The
later patches in the series do not appear to change this.

This is tagged for stable. Should the commit message describe this
behaviour change? Or should the user's MTU be kept when it still fits?

[ ... ]

> @@ -1509,17 +1524,35 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (err)
>  		return err;
>  
> +	link_changed = t->parms.link != p.link || t->fwmark != gparms.fwmark;
> +
>  	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
>  	if (err < 0)
> -		return err;
> +		goto link_update;

[ ... ]

> -	ipgre_link_update(dev, !tb[IFLA_MTU]);
> +link_update:
> +	/* ipgre_newlink_encap_setup() has published a new encapsulation even
> +	 * if ip_tunnel_changelink() failed, so the lengths must be refreshed
> +	 * on that error path as well.
> +	 *
> +	 * IFLA_MTU only defers the MTU to do_setlink(), which rtnl_changelink()
> +	 * does not reach if we return an error, so it must not hold it back.
> +	 */
> +	ipgre_link_update(dev, err || !tb[IFLA_MTU], old_hlen);

[Severity: Medium]
What happens if IFLA_MTU is present but do_setlink() then fails to apply
it?

When ip_tunnel_changelink() succeeds and tb[IFLA_MTU] is set, set_mtu is
false here. dev->mtu is left alone on the assumption that do_setlink()
will set it. However, rtnl_changelink() only calls do_setlink() after
ops->changelink() has committed the new encap and hlen, and do_setlink()
can still fail:

net/core/rtnetlink.c:do_setlink() {
    ...
	if (tb[IFLA_MTU]) {
		err = netif_set_mtu_ext(dev, nla_get_u32(tb[IFLA_MTU]), extack);
		if (err < 0)
			goto errout;
    ...
}

For example, IFLA_MTU = 0 passes the NLA_U32 policy and
validate_linkmsg(), and netif_set_mtu_ext() then rejects it. Other
do_setlink() steps that run before the MTU step can also fail.

Take a bare GRE tunnel over a 1500 byte device and add "encap fou" in
such a request. The MTU would stay at 1476 instead of 1468, which is the
stale MTU this patch sets out to fix.

A later changelink without a header change would not repair it either,
because old_hlen would already equal the new hlen.

The comment covers rtnl_changelink() not reaching do_setlink() on error.
Does it also need to handle the case where do_setlink() is reached and
fails? The same pattern also seems to be copied into erspan_changelink()
later in the series.

>  
> -	return 0;
> +	return err;
>  }

[ ... ]

> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 2a313b18134e2..dd1b2f719f216 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c

[ ... ]

> +void ip_tunnel_refresh_lengths(struct net_device *dev, bool set_mtu)
> +{
> +	int mtu = ip_tunnel_bind_dev(dev);
> +
> +	if (set_mtu)
> +		WRITE_ONCE(dev->mtu, mtu);
> +}
> +EXPORT_SYMBOL_GPL(ip_tunnel_refresh_lengths);

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Other
changelink handlers that call ip_tunnel_encap_setup() seem to have the
same stale length problem. At this commit that includes
erspan_changelink() in net/ipv4/ip_gre.c, ipip_changelink() in
net/ipv4/ipip.c and ipip6_changelink() in net/ipv6/sit.c.

For example, ipip_changelink():

net/ipv4/ipip.c:ipip_changelink() {
    ...
	if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
		int err = ip_tunnel_encap_setup(t, &ipencap);

		if (err < 0)
			return err;
    ...
	return ip_tunnel_changelink(dev, tb, &p, fwmark);
}

ip_tunnel_update() only calls ip_tunnel_bind_dev() when the link or
fwmark changed:

	if (t->parms.link != p->link || t->fwmark != fwmark) {

So "ip link set ipip0 type ipip encap fou ..." would leave dev->mtu and
dev->needed_headroom stale.

The later commit "ip_gre: recompute erspan header lengths after a change"
fixes the erspan part. It adds erspan_link_update() and calls it on both
the success and error paths.

ipip_changelink() and ipip6_changelink() still do not call
ip_tunnel_refresh_lengths() at the end of the series. Would it make
sense to use the new helper there as well?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923035217.179102-1-edumazet%40google.com

Re: [PATCH net v3 4/5] ip_gre: recompute erspan header lengths after a change

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 4 · Low: 1

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

- [Medium] The commit message says "There is no memory safety issue", but
  the bug being fixed also sends 4 uninitialized bytes of skb headroom…
- [Medium] On an existing erspan_ver 0 tunnel, every changelink that
  carries IFLA_INFO_DATA but omits IFLA_GRE_ERSPAN_VER (and…
- [Medium] In a Cc: stable fix, the patch rejects non-zero
  IFLA_GRE_IFLAGS/IFLA_GRE_OFLAGS for erspan_ver 0 and for…
- [Medium] This patch makes tunnel->tun_hlen/hlen change at runtime on a
  live erspan device (erspan_changelink() -> erspan_link_update() ->…
- [Low] erspan_xmit() now always sets IP_TUNNEL_SEQ_BIT for erspan_ver !=
  0 (line 750), whatever t->parms.o_flags holds.

--

ip_gre: recompute erspan header lengths after a change

This patch moves the ERSPAN tun_hlen/hlen computation into
erspan_set_hlen(). A new erspan_link_update() calls it at the end of
erspan_changelink() to refresh the header lengths, headroom and MTU.

It also rejects GRE flags for erspan_ver 0 and collect_md. In
erspan_xmit(), IP_TUNNEL_SEQ_BIT is now derived from erspan_ver.

> There is no memory safety issue: dev->needed_headroom is at least
> tunnel->hlen + sizeof(struct iphdr), and erspan_xmit() pushes at most
> 12 + 8 bytes before ip_tunnel_xmit() takes over and cows again.

[Severity: Medium]
Is this statement complete? The bug being fixed also appears to send 4
uninitialized bytes of skb headroom in ERSPAN packets.

In gre_build_header(), the word at greh + hdr_len - 4 is only written
when the SEQ, KEY or CSUM bit is set:

include/net/gre.h:gre_build_header() {
	skb_push(skb, hdr_len);
	...
	if (ip_tunnel_flags_intersect(flags, cond)) {
		__be32 *ptr = (__be32 *)(((u8 *)greh) + hdr_len - 4);
	...
}

Before this patch, there were two ways to reach hdr_len == 8 with none
of those bits set:

- After a v1/v2 -> v0 changelink, tun_hlen stayed at 8, but
  erspan_xmit() cleared the SEQ and KEY bits for v0.

- erspan_validate() only checks that IFLAGS | OFLAGS == GRE_SEQ | GRE_KEY.
  That means OFLAGS=KEY with IFLAGS=SEQ|KEY is accepted, and o_flags
  then lacks SEQ. erspan_xmit() clears KEY, and __gre_xmit() pushes
  8 bytes without writing any optional field.

This patch closes the first path through erspan_link_update(). It closes
the second through the new __assign_bit(IP_TUNNEL_SEQ_BIT, ...), but the
changelog only says that change makes SEQ match erspan_ver.

Since this is Cc: stable, could the changelog say that GRE header bytes
4..7 used to go out on the wire uninitialized?

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 27b3b4c584b1e..7385d66a94bf4 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -733,7 +733,6 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
>  	/* Push ERSPAN header */
>  	if (tunnel->erspan_ver == 0) {
>  		proto = htons(ETH_P_ERSPAN);
> -		__clear_bit(IP_TUNNEL_SEQ_BIT, flags);
>  	} else if (tunnel->erspan_ver == 1) {
>  		erspan_build_header(skb, ntohl(tunnel->parms.o_key),
>  				    tunnel->index,
> @@ -748,6 +747,7 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
>  		goto free_skb;
>  	}
>  
> +	__assign_bit(IP_TUNNEL_SEQ_BIT, flags, tunnel->erspan_ver != 0);

[Severity: Low]
Now that the SEQ bit is forced here for erspan_ver != 0 whatever
t->parms.o_flags holds, does the reported configuration still match what
goes out on the wire?

erspan_validate() only checks the union of IFLAGS and OFLAGS. So a v1/v2
device can be created or changed with IFLAGS=SEQ|KEY and OFLAGS=KEY
(or 0). p.o_flags is stored as is, and ipgre_fill_info() only adds the
KEY bit:

net/ipv4/ip_gre.c:ipgre_fill_info() {
	...
	if (t->erspan_ver != 0 && !t->collect_md)
		__set_bit(IP_TUNNEL_KEY_BIT, o_flags);
	...
}

The dumped IFLA_GRE_OFLAGS would then show output sequencing as off.
Yet every transmitted packet carries the GRE S bit and a sequence number.

>  	__clear_bit(IP_TUNNEL_KEY_BIT, flags);
>  	__gre_xmit(skb, dev, &tunnel->parms.iph, proto, flags);
>  	return NETDEV_TX_OK;
> @@ -1169,17 +1169,18 @@ static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
>  	if (ret)
>  		return ret;
>  
> -	if (data[IFLA_GRE_ERSPAN_VER] &&
> -	    nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0)
> -		return 0;
> -
> -	/* ERSPAN type II/III should only have GRE sequence and key flag */
>  	if (data[IFLA_GRE_OFLAGS])
>  		flags |= nla_get_be16(data[IFLA_GRE_OFLAGS]);
>  	if (data[IFLA_GRE_IFLAGS])
>  		flags |= nla_get_be16(data[IFLA_GRE_IFLAGS]);
> -	if (!data[IFLA_GRE_COLLECT_METADATA] &&
> -	    flags != (GRE_SEQ | GRE_KEY))
> +
> +	if ((data[IFLA_GRE_ERSPAN_VER] &&
> +	     nla_get_u8(data[IFLA_GRE_ERSPAN_VER]) == 0) ||
> +	    data[IFLA_GRE_COLLECT_METADATA])
> +		return flags ? -EINVAL : 0;

[Severity: Medium]
This now rejects non-zero IFLA_GRE_IFLAGS/IFLA_GRE_OFLAGS for
erspan_ver 0 and for IFLA_GRE_COLLECT_METADATA, on both newlink and
changelink. Both cases used to be accepted: ver 0 returned early, and
collect_md skipped the flags check. Is this user-visible change intended
for a stable backport? The changelog lists the new rejection but doesn't
say why it is needed.

For collect_md, erspan_fb_xmit() never reads parms.o_flags. What does
rejecting the flags there protect against?

A v1/v2 device's own dump also no longer seems replayable with a
different version. ipgre_fill_info() reports SEQ|KEY for v1/v2 (KEY is
forced, as shown above). A request with IFLA_GRE_ERSPAN_VER=0 plus those
flags now fails with -EINVAL.

If a client resends the existing flags and has no way to clear SEQ, can
a v1/v2 -> v0 changelink still be done, or does the device have to be
recreated? I haven't checked iproute2's exact behaviour here.

> +
> +	/* ERSPAN type II/III should only have GRE sequence and key flag */
> +	if (flags != (GRE_SEQ | GRE_KEY))
>  		return -EINVAL;
>  
>  	/* ERSPAN Session ID only has 10-bit. Since we reuse
> @@ -1317,7 +1318,11 @@ static int erspan_netlink_parms(struct net_device *dev,
>  			return -EINVAL;
>  	}
>  
> -	if (gparms->erspan_ver == 1) {
> +	if (gparms->erspan_ver == 0) {
> +		if (!ip_tunnel_flags_empty(parms->i_flags) ||
> +		    !ip_tunnel_flags_empty(parms->o_flags))
> +			return -EINVAL;

[Severity: Medium]
Can a changelink on an existing erspan_ver 0 device still succeed if it
doesn't resend IFLA_GRE_ERSPAN_VER?

erspan_validate() has no view of the device state. Without
IFLA_GRE_ERSPAN_VER (and without IFLA_GRE_COLLECT_METADATA), it skips
the new early return and requires:

	if (flags != (GRE_SEQ | GRE_KEY))
		return -EINVAL;

A request with no flags attributes has flags == 0, so it fails there too.

erspan_netlink_parms() then inherits gparms->erspan_ver from
t->erspan_ver (0). This new branch rejects any non-empty i_flags or
o_flags, which is exactly what erspan_validate() just required.

So on a v0 device, no request that carries IFLA_INFO_DATA without
IFLA_GRE_ERSPAN_VER seems able to pass both checks. Before this patch,
the same request with SEQ|KEY was accepted. Is that intended?

> +	} else if (gparms->erspan_ver == 1) {
>  		if (data[IFLA_GRE_ERSPAN_INDEX]) {
>  			gparms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
>  			if (gparms->index & ~INDEX_MASK)

[ ... ]

> @@ -1575,15 +1607,30 @@ static int erspan_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (err)
>  		return err;
>  
> +	link_changed = t->parms.link != p.link || t->fwmark != gparms.fwmark;
> +
>  	err = ip_tunnel_changelink(dev, tb, &p, gparms.fwmark);
>  	if (err < 0)
> -		return err;
> +		goto link_update;
> +
> +	if (link_changed)
> +		old_hlen = t->hlen;
>  
>  	ipgre_commit_parms(t, &gparms);
>  	ip_tunnel_flags_copy(t->parms.i_flags, p.i_flags);
>  	ip_tunnel_flags_copy(t->parms.o_flags, p.o_flags);
>  
> -	return 0;
> +link_update:
> +	/* ipgre_newlink_encap_setup() has published a new encapsulation
> +	 * without the ERSPAN header length even if ip_tunnel_changelink()
> +	 * failed, so the lengths must be refreshed on that error path too.
> +	 *
> +	 * As in ipgre_changelink(), IFLA_MTU must not hold the MTU back if we
> +	 * return an error, because do_setlink() will not apply it then.
> +	 */
> +	erspan_link_update(dev, err || !tb[IFLA_MTU], old_hlen);

[Severity: Medium]
Now that tun_hlen and hlen change at runtime on a live device, what
orders these updates against erspan_xmit()?

erspan_changelink() does plain stores in this order:

- t->erspan_ver, in ipgre_commit_parms()
- t->parms.o_flags
- tun_hlen and hlen, in erspan_set_hlen()
- dev->needed_headroom, in ip_tunnel_bind_dev()

There is no netif_tx_disable(), no synchronize_net() and no
READ_ONCE()/WRITE_ONCE().

erspan_xmit() runs without RTNL and reads these values one after
another:

erspan_xmit()
    skb_cow_head(skb, dev->needed_headroom)
    ip_tunnel_flags_copy(flags, tunnel->parms.o_flags)
    tunnel->erspan_ver       (if/else chain, ERSPAN header)
    tunnel->erspan_ver       (new __assign_bit())
    __gre_xmit()
        gre_build_header(skb, tunnel->tun_hlen, ...)

Each read can see a different generation of the configuration:

- 0 -> 2: SEQ is set from the new version while tun_hlen is still 4. The
  sequence number then overwrites greh->flags and greh->protocol, which
  is the corruption this patch fixes, now as a transient window.

- 2 -> 0: SEQ is cleared while tun_hlen is still 8. 8 bytes are pushed
  but only 4 are written, so 4 bytes of stale headroom go on the wire.

- The ERSPAN header and the SEQ bit can come from different versions.

- The skb may have been cowed for the old, smaller needed_headroom while
  up to 20 + 8 bytes get pushed. With an underlay whose hard_header_len
  + needed_headroom is below 4, could skb_push() reach
  skb_under_panic()? That seems to need very little headroom slack, but
  it would contradict the "no memory safety issue" statement.

Would it close this window to take one READ_ONCE() snapshot of
erspan_ver in erspan_xmit(), derive both the GRE length and the SEQ bit
from it, and cow for the size actually pushed? A similar o_flags vs
tun_hlen race seems to exist already for plain GRE in
ipgre_link_update().

> +
> +	return err;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923035217.179102-1-edumazet%40google.com

Re: [PATCH net v3 5/5] gre: do not read inner frame as erspan metadata in collect_md mode

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · 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).

- [Medium] The fix is incomplete on the IPv6 side.

--

gre: do not read inner frame as erspan metadata in collect_md mode

In collect_md mode, erspan_rcv() and ip6erspan_rcv() could copy bytes
from the inner Ethernet frame into the ERSPAN metadata. This happened for
type I packets and for packets whose base header version was not 1 or 2.
The patch rejects base header versions other than 1 and 2, skips metadata
extraction for type I, and makes is_erspan_type1() match the rule in
gre_parse_header().

> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index e61cb10b50dc9..774975955747e 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -503,6 +503,8 @@ static int ip6erspan_rcv(struct sk_buff *skb,
>  	ipv6h = ipv6_hdr(skb);
>  	ershdr = (struct erspan_base_hdr *)skb->data;
>  	ver = ershdr->ver;
> +	if (unlikely(ver != 1 && ver != 2))
> +		return PACKET_REJECT;

[Severity: Medium]
Can a 4-byte ETH_P_ERSPAN (type I) GRE frame over IPv6 still get past this
check, so that inner Ethernet bytes end up in the metadata?

gre_parse_header() treats a 4-byte ETH_P_ERSPAN header as type I. It does
not parse an ERSPAN header in that case:

net/ipv4/gre_demux.c:gre_parse_header() {
    ...
	if ((greh->protocol == htons(ETH_P_ERSPAN) && hdr_len != 4) ||
	    greh->protocol == htons(ETH_P_ERSPAN2)) {
    ...
}

gre_rcv() in ip6_gre.c then passes every ETH_P_ERSPAN frame to
ip6erspan_rcv(), including the 4-byte ones:

	if (unlikely(tpi.proto == htons(ETH_P_ERSPAN) ||
		     tpi.proto == htons(ETH_P_ERSPAN2))) {
		if (ip6erspan_rcv(skb, &tpi, hdr_len) == PACKET_RCVD)

For such a frame, skb->data points at the inner destination MAC. That means
ershdr->ver is the high nibble of the first MAC byte. If that byte is 0x1X
or 0x2X, the new check passes.

ip6gre_tunnel_lookup() can then fall back to collect_md_tun_erspan. In that
case, erspan_hdr_len(ver) pulls 8 or 12 bytes of the inner Ethernet header.
pkt_md then points at inner frame offset 4, and this copies MAC address
bytes into md->u:

			pkt_md = (struct erspan_metadata *)(gh + gre_hdr_len +
							    sizeof(*ershdr));
			md = ip_tunnel_info_opts(info);
			md->version = ver;
			md2 = &md->u.md2;
			memcpy(md2, pkt_md, ver == 1 ? ERSPAN_V1_MDSIZE :
						       ERSPAN_V2_MDSIZE);

A frame whose first MAC nibble is 0 is now rejected, but one whose nibble
is 1 or 2 is delivered truncated, with metadata built from the MAC bytes.

The IPv4 side now follows the gre_parse_header() rule through
is_erspan_type1(). ip6erspan_tap_validate() only accepts erspan_ver 1 or 2,
so type I is never valid on IPv6.

Should ip6erspan_rcv() reject tpi->proto == htons(ETH_P_ERSPAN) &&
gre_hdr_len == 4 before it reads ershdr->ver?

>  
>  	tunnel = ip6gre_tunnel_lookup(skb->dev,
>  				      &ipv6h->saddr, &ipv6h->daddr, tpi->key,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923035217.179102-1-edumazet%40google.com

Re: [PATCH net v3 1/5] ip_tunnel: do not clear the active encap before validating the new one

[Eric Dumazet]

On Fri, Sep 25, 2026 at 8:53 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · 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).
>
> - [Medium] The patch only fixes the case where
>   ip_tunnel_encap_setup()/ip6_tnl_encap_setup() itself fails.

Right, this patch is only about the memset(): a request that
ip[6]_encap_hlen() rejects must leave the tunnel alone.

ipip, sit, ip6_tunnel and ip6_gre changelink handlers setting up
the encapsulation before validating the rest of the request is an older,
separate issue. Fixing it requires staging the encapsulation,
as patches 3 and 4 do for ip_gre, in each of these drivers.
This can be handled separately; v4 states it in the changelog.


>
> Pre-existing issues:
> - [Medium] ip_tunnel_encap_setup() and ip6_tnl_encap_setup() update
>   t->encap.{type,sport,dport,flags}, t->encap_hlen and t->hlen one field…
>

Pre-existing indeed, and not specific to the encap fields: the xmit
paths read o_flags,
tun_hlen, needed_headroom and friends locklessly as well.
READ_ONCE()/WRITE_ONCE() annotations (as started by 88b84cae6b94 for IPv4),
or a consistent snapshot of the configuration in the xmit paths, are
net-next material.

In fact, my plan is to convert everything to RCU.

pw-bot: cr

Re: [PATCH net v3 3/5] ip_gre: compute tunnel lengths absolutely instead of by delta

[Eric Dumazet]

On Fri, Sep 25, 2026 at 8:53 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 (1) · 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).
>
> - [Medium] When the GRE header length changes, ipgre_link_update() now
>   replaces the user-configured dev->mtu with the default computed by…

Agreed. v4 shifts dev->mtu by the header length difference again,
now computed against the tunnel->hlen sampled on entry, and drops
ip_tunnel_refresh_lengths().


> - [Medium] A changelink that publishes a new header length but carries an
>   IFLA_MTU that do_setlink() later rejects leaves the tunnel with the…

v4 shifts the MTU even when IFLA_MTU is present.

>
> Pre-existing issues:
> - [Medium] Sibling changelink handlers publish a new encap_hlen through
>   ip_tunnel_encap_setup() but never recompute dev->mtu or…
>

erspan is fixed by patch 4. ipip and sit are not touched by this
series, see my reply to patch 1: this can be handled separately.

Re: [PATCH net v3 4/5] ip_gre: recompute erspan header lengths after a change

[Eric Dumazet]

On Fri, Sep 25, 2026 at 8:53 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 4 · Low: 1
>
> 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).
>
> - [Medium] The commit message says "There is no memory safety issue", but
>   the bug being fixed also sends 4 uninitialized bytes of skb headroom…

Right, the v4 changelog documents both ways of sending 4 bytes of
uninitialized headroom, and the GRE_CSUM corruption on version 0.

> - [Medium] On an existing erspan_ver 0 tunnel, every changelink that
>   carries IFLA_INFO_DATA but omits IFLA_GRE_ERSPAN_VER (and…
> - [Medium] In a Cc: stable fix, the patch rejects non-zero
>   IFLA_GRE_IFLAGS/IFLA_GRE_OFLAGS for erspan_ver 0 and for…

Agreed, v4 drops the new validation. erspan_xmit() builds the GRE
flags from the version alone instead, so existing configurations keep
working.

> - [Medium] This patch makes tunnel->tun_hlen/hlen change at runtime on a
>   live erspan device (erspan_changelink() -> erspan_link_update() ->…

erspan_xmit() racing with a changelink is not new. Before this patch,
such a changelink left tun_hlen and hlen out of sync with the version
for good, and a new encapsulation was not accounted for in
dev->needed_headroom and dev->mtu. Plain GRE has had the same o_flags
vs tun_hlen window since dd9d598c6657 ("ip_gre: add the support for
i/o_flags update via netlink"). v4 narrows the erspan case to the
duration of the changelink.

On the headroom side, the version change alone makes erspan_xmit()
push at most 12 bytes more than tunnel->hlen accounts for, which the
20 bytes reserved for the outer IP header in dev->needed_headroom
absorb. Going further needs a concurrent encapsulation change, whose
header ip_tunnel_encap() pushes before ip_tunnel_xmit() cows again:
that is the pre-existing race discussed in patch 1, common to all
ip_tunnel users.

Closing these windows needs a snapshot of the tunnel configuration in
the transmit paths of all GRE flavors, which is out of scope for this
series.

RCU conversion will be done in net-next.