Re: [PATCH nf] netfilter: flowtable: fix IP6IP6 tunnel offset double-count with vlan/pppoe encap

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

> nf_flow_ip6_tunnel_proto() stores the return value of ipv6_skip_exthdr()
> directly into ctx->tun.hdr_size and then does ctx->offset +=
> ctx->tun.hdr_size.
> 
> ipv6_skip_exthdr() returns an offset measured from skb->data, i.e. its
> result already includes the "sizeof(*ip6h) + ctx->offset" start argument.
> So hdr_size ends up containing ctx->offset, and the subsequent
> "ctx->offset += ctx->tun.hdr_size" counts the encap offset twice.
> 
> This is harmless for a bare IP6IP6 packet, where ctx->offset is 0 on
> entry, which is why it has gone unnoticed. But nf_flow_skb_encap_protocol()
> advances ctx->offset by VLAN_HLEN / PPPOE_SES_HLEN before the tunnel
> parser runs, so for an IP6IP6 flow carried over vlan or pppoe both
> ctx->offset and ctx->tun.hdr_size are off by the encap length:
> 
>   - nf_flow_tuple_ipv6() then reads the inner header at the wrong offset,
>     the computed tuple no longer matches the flowtable entry, and the
>     packet silently falls back to the slow path (IP6IP6 rx acceleration
>     stops working);
>   - on the forward path nf_flow_ip_tunnel_pop() would skb_pull() past the
>     inner header.
> 
> The IPv4 sibling nf_flow_ip4_tunnel_proto() does this correctly: it stores
> a relative header length (iph->ihl << 2) and adds that to ctx->offset.
> Make the IPv6 path symmetric by storing the relative size.
> 
> Fixes: d98103575dcd ("netfilter: flowtable: Add IP6IP6 rx sw acceleration")
> Signed-off-by: David Carlier <devnexen@gmail.com>

Hi David,

thx for fixing it. I developed the IP6IP6 vlan support using the veth as
underlying device. veth enables vlan rx/tx offload by default so I was
not able to spot the issue.

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

> ---
>  net/netfilter/nf_flow_table_ip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index 9c05a50d6013..4c6a68765c6b 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -366,7 +366,7 @@ static bool nf_flow_ip6_tunnel_proto(struct nf_flowtable_ctx *ctx,
>  		return false;
>  
>  	if (nexthdr == IPPROTO_IPV6) {
> -		ctx->tun.hdr_size = hdrlen;
> +		ctx->tun.hdr_size = hdrlen - ctx->offset;
>  		ctx->tun.proto = IPPROTO_IPV6;
>  	}
>  	ctx->offset += ctx->tun.hdr_size;
> -- 
> 2.53.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]