Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: Kernel 5.0-rc5 regression with NAT, bisected to: netfilter: nat:
 remove l4proto->manip_pkt
To:     Florian Westphal <fw@strlen.de>
Cc:     Pablo Neira Ayuso <pablo@netfilter.org>,
        "David S. Miller" <davem@davemloft.net>,
        netdev <netdev@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
References: <40b70892-daf5-28d7-28b5-869911faf2bb@eikelenboom.it>
 <20190208070710.rcbj6exqwz6m2o7o@breakpoint.cc>
 <20190208115447.ojyfhenf44kqs3w4@breakpoint.cc>
From:   Sander Eikelenboom <linux@eikelenboom.it>
Message-ID: <0c5ac167-7682-a945-d402-19f2e023fc5f@eikelenboom.it>
Date:   Fri, 8 Feb 2019 16:34:32 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <20190208115447.ojyfhenf44kqs3w4@breakpoint.cc>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 08/02/2019 12:54, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
>> Sander Eikelenboom <linux@eikelenboom.it> wrote:
>>> L.S.,
>>>
>>> While trying out a 5.0-RC5 kernel I seem to have stumbled over a regression with NAT.
>>> (using an nftables firewall with NAT and connection tracking).
>>>
>>> Unfortunately it isn't too obvious since no errors are logged, but on clients it
>>> causes symptoms like firefox intermittently not being able to load pages with:
>>>     Network Protocol Error
>>>     An error occurred during a connection to www.example.com
>>>     The page you are trying to view cannot be shown because an error in the network protocol was detected.
>>>     Please contact the website owners to inform them of this problem.
>>>
>>> But it's only intermittently, so i can still visit some webpages with clients, 
>>> could be that packet size and or fragments are at play ?
>>>
>>> So I tried testing with git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git with 
>>> e8c32c32b48c2e889704d8ca0872f92eb027838e as last commit, to be sure to have the latest netdev has to offer,
>>> but to no avail. 
>>>
>>> After that I tried to git bisect and ended up with:
>>>
>>> faec18dbb0405c7d4dda025054511dc3a6696918 is the first bad commit
>>> commit faec18dbb0405c7d4dda025054511dc3a6696918
>>> Author: Florian Westphal <fw@strlen.de>
>>> Date:   Thu Dec 13 16:01:33 2018 +0100
>>>
>>>     netfilter: nat: remove l4proto->manip_pkt
>>
>> Thanks, this is immensely helpful.
>>
>> I think I see the bug, we can't use target->dst.protonum in
>> nf_nat_l4proto_manip_pkt(), it will be TCP in case we're dealing
>> with a related icmp packet.
>>
>> I will send a patch in a few hours when I get back.
> 
> Sander, does this patch fix things for you?

Hi Florian,

You may stick on a reported/tested-by if you like.
Thanks for the swift fix !

--
Sander

> 
> Thanks!
> 
> diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
> --- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
> @@ -215,6 +215,7 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb,
>  
>  	/* Change outer to look like the reply to an incoming packet */
>  	nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple);
> +	target.dst.protonum = IPPROTO_ICMP;
>  	if (!nf_nat_ipv4_manip_pkt(skb, 0, &target, manip))
>  		return 0;
>  
> diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
> --- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
> +++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
> @@ -226,6 +226,7 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb,
>  	}
>  
>  	nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple);
> +	target.dst.protonum = IPPROTO_ICMPV6;
>  	if (!nf_nat_ipv6_manip_pkt(skb, 0, &target, manip))
>  		return 0;
>  
>