Re: [PATCH net] net: gro: compare skb->protocol in gro_list_prepare()

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Eric Dumazet wrote:
> gro_list_prepare() checks whether a held packet may belong to the same
> flow as the incoming one by comparing the device, VLAN tag, metadata
> and the link-layer header, using dev->hard_header_len as its length.
> 
> On devices without a link-layer header (hard_header_len == 0), such as
> L3 GRE tunnels, no compared byte carries the outer protocol:
> skb->protocol comes from the tunnel header. Packets of different
> protocols can then stay eligible, and the GRO handler selected from the
> incoming skb->protocol parses the held packet with the wrong layout.
> 
> For example, take an IPv4/UDP packet held by UDP GRO and a
> VLAN/IPv4/TCP packet received on the same ipgre device.
> vlan_gro_receive() reads the first four bytes of the held IPv4 header
> as a VLAN header. With crafted values, the held packet survives up to
> tcp_gro_receive(), which uses its UDP header as a TCP header. With a
> short linear part, the FIN/PSH update of the held packet's TCP flags
> can then write into skb_shared_info.
> 
> Nested GRO handlers (VLAN, GRE, IPv4, IPv6) already compare their own
> next-protocol field. Only the outermost protocol was not checked.
> 
> Compare skb->protocol too. On Ethernet devices this is redundant with
> compare_ether_header(), but it is cheap.
> 
> Fixes: 60769a5dcd87 ("ipv4: gre: add GRO capability")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Signed-off-by: Eric Dumaze <edumazet@kernel.org>

Reviewed-by: Willem de Bruijn <willemb@google.com>