Re: [PATCH net v2 2/2] geneve: validate inner network offset in geneve_gro_complete()

[Paolo Abeni <pabeni@redhat.com>]

On 6/9/26 6:13 AM, Xiang Mei wrote:
> Even with both paths gated on gs->gro_hint, geneve_gro_complete()
> re-derives the inner dispatch type and length from the packet and the
> current gs->gro_hint, independently of geneve_gro_receive(). The two can
> disagree if gs->gro_hint flips under a concurrent geneve_quiesce()/
> geneve_unquiesce() (sk_user_data is NULL across a synchronize_net()), or if
> the re-read option bytes differ from the ones receive parsed.
> 
> geneve_gro_receive() already records the inner network header position in
> NAPI_GRO_CB()->inner_network_offset. Have geneve_gro_complete() check the
> offset it is about to dispatch at against that value, adding ETH_HLEN in
> the ETH_P_TEB case where eth_gro_complete() steps over the inner MAC
> header, and bail out on mismatch instead of trusting the re-derivation.
> 
> Fixes: fd0dd796576e ("geneve: use GRO hint option in the RX path")
> Assisted-by: Claude:claude-opus-4-8
> Tested-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
> v2: Add patch for race condition found by Sashiko
> 
>  drivers/net/geneve.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index d0dc5d6c46df..028740e97740 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -956,6 +956,19 @@ static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,
>  	type = gh->proto_type;
>  	geneve_sk_gro_hint_off(sk, gh, &type, &gh_len);
>  
> +	/* Bail out if our inner network offset disagrees with gro_receive().
> +	 * ETH_P_TEB adds ETH_HLEN for the inner MAC header.
> +	 */
> +	if (skb->encapsulation) {

I think the disagreement could happen even in the opposite direction,
i.e. gro_receives does not see hints available, but gro_complete does.

> +		unsigned int inner_nh = nhoff + gh_len;
> +
> +		if (type == htons(ETH_P_TEB))
> +			inner_nh += ETH_HLEN;

This does not work in when the innermost headers carry a vlan tag.

/P