netdev - Re: [PATCH net] udp: fix a potential panic in first_packet

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: <1486659678.2591.11.camel@redhat.com>
Date:   Thu, 09 Feb 2017 18:01:18 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH net] udp: fix a potential panic in first_packet_length()

On Thu, 2017-02-09 at 07:30 -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@...gle.com>
> 
> first_packet_length() is called from udp_ioctl()
> 
> udp_ioctl(), as its name suggests, is used by UDP protocols,
> but is also used by L2TP :(
> 
> We shall call udp_rmem_release() only for UDP variants.
> 
> Thanks to Andrey and syzkaller team for providing the report
> and a nice reproducer.
> 
> Fixes: 7c13f97ffde63 ("udp: do fwd memory scheduling on dequeue")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
> ---
>  net/ipv4/udp.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 8aab7d78d25bc6eaa42dcc960cdbd5086f614cad..7c0807ee82cec6ca8c856da14fa6109dfdf27868 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1376,7 +1376,11 @@ static int first_packet_length(struct sock *sk)
>  		kfree_skb(skb);
>  	}
>  	res = skb ? skb->len : -1;
> -	if (total)
> +	/* udp_ioctl() can be used by UDP/UDPLite, but also L2TP.
> +	 * We only need to call udp_rmem_release() for UDP sockets.
> +	 * L2TP does have a proper skb destructor invoked at kfree_skb() time.
> +	 */
> +	if (total && sk->sk_prot->memory_allocated == &udp_memory_allocated)
>  		udp_rmem_release(sk, total, 1);
>  	spin_unlock_bh(&rcvq->lock);
>  	return res;
> 
> 

My bad, I missed completely that call path.

I'm wondering if calling first_packet_length() for l2tp_ip sockets
makes sense ?!? Am I missing something or it touches udp stats and
checks udp csum for non udp packets ?!?

Paolo