netdev - Re: [PATCH net 1/2] xfrm6: Fix IPv6 payload_len in xfrm6_transport

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: <20170621053725.GP2631@secunet.com>
Date:   Wed, 21 Jun 2017 07:37:26 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     <yossiku@...lanox.com>
CC:     <netdev@...r.kernel.org>, Herbert Xu <herbert@...dor.apana.org.au>,
        Yevgeny Kliteynik <kliteyn@...lanox.com>,
        Boris Pismenny <borisp@...lanox.com>,
        Ilan Tayari <ilant@...lanox.com>
Subject: Re: [PATCH net 1/2] xfrm6: Fix IPv6 payload_len in
 xfrm6_transport_finish

On Mon, Jun 19, 2017 at 11:33:20AM +0300, yossiku@...lanox.com wrote:
> From: Yossi Kuperman <yossiku@...lanox.com>
> 
> IPv6 payload length indicates the size of the payload, including any
> extension headers. In xfrm6_transport_finish, ipv6_hdr(skb)->payload_len
> is set to the payload size only, regardless of the presence of any
> extension headers.
> 
> After ESP GRO transport mode decapsulation, ipv6_rcv trims the packet
> according to the wrong payload_len, thus corrupting the packet.
> 
> Set payload_len to account for extension headers as well.
> 
> Fixes: 716062fd4c2f ("[IPSEC]: Merge most of the input path")
> Signed-off-by: Yossi Kuperman <yossiku@...lanox.com>
> ---
>  net/ipv6/xfrm6_input.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
> index 08a807b..3ef5d91 100644
> --- a/net/ipv6/xfrm6_input.c
> +++ b/net/ipv6/xfrm6_input.c
> @@ -43,8 +43,8 @@ int xfrm6_transport_finish(struct sk_buff *skb, int async)
>  		return 1;
>  #endif
>  
> -	ipv6_hdr(skb)->payload_len = htons(skb->len);
>  	__skb_push(skb, skb->data - skb_network_header(skb));
> +	ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(struct ipv6hdr));

You mentioned that the bug happens with ESP GRO. Does this bug also
happen in the standard codepath? If not, you might better move the
above line into the 'if' section below.

>  
>  	if (xo && (xo->flags & XFRM_GRO)) {
>  		skb_mac_header_rebuild(skb);
> -- 
> 2.8.1