netdev - Re: [PATCH net] vti6: remove !skb->ignore_df check from vti6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180829083928.63gfafxg7ccfvype@gauss3.secunet.de>
Date:   Wed, 29 Aug 2018 10:39:28 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Alexey Kodanev <alexey.kodanev@...cle.com>
CC:     <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: [PATCH net] vti6: remove !skb->ignore_df check from vti6_xmit()

On Thu, Aug 23, 2018 at 07:49:54PM +0300, Alexey Kodanev wrote:
> Before the commit d6990976af7c ("vti6: fix PMTU caching and reporting
> on xmit") '!skb->ignore_df' check was always true because the function
> skb_scrub_packet() was called before it, resetting ignore_df to zero.
> 
> In the commit, skb_scrub_packet() was moved below, and now this check
> can be false for the packet, e.g. when sending it in the two fragments,
> this prevents successful PMTU updates in such case. The next attempts
> to send the packet lead to the same tx error. Moreover, vti6 initial
> MTU value relies on PMTU adjustments.
> 
> This issue can be reproduced with the following LTP test script:
>     udp_ipsec_vti.sh -6 -p ah -m tunnel -s 2000
> 
> Fixes: ccd740cbc6e0 ("vti6: Add pmtu handling to vti6_xmit.")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@...cle.com>
> ---
> Not sure about xfrmi_xmit2(), it has a similar check for ignore_df...
> 
>  net/ipv6/ip6_vti.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index 38dec9d..f48d196 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -481,7 +481,7 @@ static bool vti6_state_check(const struct xfrm_state *x,
>  	}
>  
>  	mtu = dst_mtu(dst);
> -	if (!skb->ignore_df && skb->len > mtu) {
> +	if (skb->len > mtu) {
>  		skb_dst_update_pmtu(skb, mtu);

This looks OK to me. If I remember correct, the !skb->ignore_df
check was taken from the native xfrm6 PMTU handling. There this 
check makes sense because the packet can be still fragmented
along the way through the stack. In this case here it is too late
as we are about to TX the packet through the vti device. So
we should update to the new IPsec PMTU and notify the sender
about this.

Acked-by: Steffen Klassert <steffen.klassert@...unet.com>