netdev - Re: [PATCH 1/2] xfrm6: call kfree

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: <20180831064712.GF23674@gauss3.secunet.de>
Date:   Fri, 31 Aug 2018 08:47:12 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Sabrina Dubroca <sd@...asysnail.net>
CC:     Thadeu Lima de Souza Cascardo <cascardo@...onical.com>,
        <netdev@...r.kernel.org>, <yoshfuji@...ux-ipv6.org>,
        <kuznet@....inr.ac.ru>, <davem@...emloft.net>,
        <herbert@...dor.apana.org.au>, <eyal.birger@...il.com>
Subject: Re: [PATCH 1/2] xfrm6: call kfree_skb when skb is toobig

On Thu, Aug 30, 2018 at 03:23:11PM +0200, Sabrina Dubroca wrote:
> 2018-08-30, 09:58:16 -0300, Thadeu Lima de Souza Cascardo wrote:
> > After commit d6990976af7c5d8f55903bfb4289b6fb030bf754 ("vti6: fix PMTU caching
> > and reporting on xmit"), some too big skbs might be potentially passed down to
> > __xfrm6_output, causing it to fail to transmit but not free the skb, causing a
> > leak of skb, and consequentially a leak of dst references.
> > 
> > After running pmtu.sh, that shows as failure to unregister devices in a namespace:
> > 
> > [  311.397671] unregister_netdevice: waiting for veth_b to become free. Usage count = 1
> > 
> > The fix is to call kfree_skb in case of transmit failures.
> > 
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@...onical.com>

Good catch!

> Reviewed-by: Sabrina Dubroca <sd@...asysnail.net>
> 
> I was about to post the same patch. Arguably, the commit introducing
> this bug is the one that added those "return -EMSGSIZE" to
> __xfrm6_output without freeing.
> 
> Either way, it's missing a Fixes: tag, which should be one of those,
> or both:
> 
> Fixes: d6990976af7c ("vti6: fix PMTU caching and reporting on xmit")
> Fixes: dd767856a36e ("xfrm6: Don't call icmpv6_send on local error")

This bug can be triggered even without vti6, so the correct
Fixes tag would be the latter.

Thadeu, please resend this one with the Fixes tag.

Thanks!