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]
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>

Powered by blists - more mailing lists