[PATCH net-next v2] erspan: drop packets with an unknown ERSPAN version

[PATCH net-next v2] erspan: drop packets with an unknown ERSPAN version

[Haishuang Yan]

The ERSPAN type II and III receive paths take the length of the ERSPAN
header from the version field of the packet, with erspan_hdr_len(), but
never check that the version is one they know: 1 for type II or 2 for
type III.

For any other version, erspan_hdr_len() returns 0 or the type III
length. With version 0, the ERSPAN header is not pulled, so it is
passed up as the start of the inner Ethernet frame, and in collect_md
mode the metadata is filled from the wrong part of the header. The
ETH_HLEN pull in __iptunnel_pull_header() still covers the bytes that
are copied, so nothing is read past the pulled data.

Reject such packets, as a packet with a truncated ERSPAN header already
is.

Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
Tested with hand-built GRE/ERSPAN packets sent over a veth into erspan
and ip6erspan devices, in plain and collect_md mode: version 0 packets
are now dropped, version 1 and 2 packets are unchanged.

v2:
- Drop the Fixes tags and correct the commit message: the ETH_HLEN pull
  of __iptunnel_pull_header() covers the metadata copy, so there is no
  out of bounds read (Ido, Sashiko).
- Drop the patch that skipped the type III sub-header, as no device is
  known to send it (Ido).
v1: https://lore.kernel.org/netdev/20261018090236.2487997-1-yanhaishuang@cmss.chinamobile.com/

 net/ipv4/ip_gre.c  | 4 +++-
 net/ipv6/ip6_gre.c | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 5485586fa0e6..ea41be3c4229 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -303,8 +303,10 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 	if (tunnel) {
 		if (is_erspan_type1(gre_hdr_len))
 			len = gre_hdr_len;
-		else
+		else if (ver == ERSPAN_VERSION || ver == ERSPAN_VERSION2)
 			len = gre_hdr_len + erspan_hdr_len(ver);
+		else
+			return PACKET_REJECT;
 
 		if (unlikely(!pskb_may_pull(skb, len)))
 			return PACKET_REJECT;
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index b5d6ed8f0c36..46fac89a0743 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -507,6 +507,9 @@ static int ip6erspan_rcv(struct sk_buff *skb,
 	if (tunnel) {
 		int len = erspan_hdr_len(ver);
 
+		if (unlikely(ver != ERSPAN_VERSION && ver != ERSPAN_VERSION2))
+			return PACKET_REJECT;
+
 		if (unlikely(!pskb_may_pull(skb, len)))
 			return PACKET_REJECT;
 

base-commit: 303d91a1592587fdb481dd36a9ef2c0414dacb0f
-- 
2.43.0

Re: [PATCH net-next v2] erspan: drop packets with an unknown ERSPAN version

[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 new version check can be bypassed: an ETH_P_ERSPAN2 (type
  III) frame with a 4-byte GRE header never reaches it.

--

erspan: drop packets with an unknown ERSPAN version

This patch makes erspan_rcv() and ip6erspan_rcv() reject packets whose
ERSPAN version field is not 1 (type II) or 2 (type III). Before this,
such packets either had no ERSPAN header pulled or were given the type
III length.

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 5485586fa0e62..ea41be3c4229b 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -303,8 +303,10 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>  	if (tunnel) {
>  		if (is_erspan_type1(gre_hdr_len))
>  			len = gre_hdr_len;
> -		else
> +		else if (ver == ERSPAN_VERSION || ver == ERSPAN_VERSION2)
>  			len = gre_hdr_len + erspan_hdr_len(ver);
> +		else
> +			return PACKET_REJECT;

[Severity: Medium]
Can an ETH_P_ERSPAN2 frame with a 4-byte GRE header get around this new
check?

is_erspan_type1() looks only at gre_hdr_len and never checks
tpi->proto:

net/ipv4/ip_gre.c:is_erspan_type1() {
	/* 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;
}

gre_parse_header() treats ETH_P_ERSPAN2 (0x22EB, defined in uAPI as
type III) as always carrying an ERSPAN base header, whatever hdr_len
is:

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

gre_rcv() then calls erspan_rcv(skb, &tpi, 4). Since
is_erspan_type1(4) is true, ver is forced to 0 and the lookup uses
NO_KEY with key 0. That can match a keyless erspan_ver 0 tunnel or the
collect_md fallback in ip_tunnel_lookup().

>From there len = gre_hdr_len and the new version check is skipped. An
ERSPAN header with any version (0 or 3..15) is passed up as the start
of the inner Ethernet frame, which is the outcome the commit message
says is fixed.

In collect_md mode, tun_id = key32_to_tunnel_id(tpi->key) is also built
from the session ID in those 4 bytes, and the same bytes are handed up
as the inner MAC header.

__iptunnel_pull_header() still pulls ETH_HLEN, so nothing is read out
of bounds.

Would it make sense to also require tpi->proto == htons(ETH_P_ERSPAN)
when classifying a packet as type I, so this path is covered too?

>  
>  		if (unlikely(!pskb_may_pull(skb, len)))
>  			return PACKET_REJECT;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107130030.35220-1-yanhaishuang%40cmss.chinamobile.com