Re: [PATCH net-next v3] 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] On IPv6, the new ERSPAN version check in ip6erspan_rcv()
  (net/ipv6/ip6_gre.c) can read its version from the wrong bytes, so…

--

erspan: drop packets with an unknown ERSPAN version

This patch makes the IPv4 and IPv6 ERSPAN receive paths drop packets
whose ERSPAN version is not 1 (type II) or 2 (type III). On IPv4, it
also checks for ETH_P_ERSPAN when classifying a packet as type I, so a
4-byte-GRE ETH_P_ERSPAN2 packet now gets the version check too.

> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index b5d6ed8f0c369..46fac89a07437 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;
> +

[Severity: Medium]

Can some type I packets still get through this check on IPv6?

gre_parse_header() treats an ETH_P_ERSPAN packet with a 4-byte GRE
header as type I, with no ERSPAN header. In that case it leaves
tpi->key at 0:

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

The IPv6 gre_rcv() pulls the GRE header first. It then passes every
ETH_P_ERSPAN and ETH_P_ERSPAN2 packet to ip6erspan_rcv(), which has no
type I branch and reads the version straight from skb->data:

	ershdr = (struct erspan_base_hdr *)skb->data;
	ver = ershdr->ver;

For a 4-byte-GRE ETH_P_ERSPAN packet, ver is the high nibble of the
first byte of the inner Ethernet destination MAC. If that nibble is 1
or 2:

- the new check passes
- erspan_hdr_len(ver) strips 8 or 12 bytes of the real inner frame
- the tunnel lookup uses key 0 instead of a session ID
- in collect_md mode, erspan_metadata is filled from inner frame bytes

The commit message describes the version 0 case like this:

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

This is the same symptom, with the offset wrong in the other
direction. Whether such a packet is dropped now depends on the inner
destination MAC. Nibbles 0 and 3-15 are rejected. Nibbles 1 and 2 are
accepted, and the frame is passed up from the wrong offset.

In v3, the IPv4 side was brought in line with gre_parse_header() by
checking tpi->proto in is_erspan_type1(). Would ip6erspan_rcv() need a
matching change? It could reject packets where tpi->proto ==
htons(ETH_P_ERSPAN) && gre_hdr_len == 4, since ip6erspan_tap_validate()
only allows versions 1 and 2 for IPv6 ERSPAN devices.

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

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