Re: net/icmp: null-ptr-deref in ping_v4_push_pending_frames
From: Florian Westphal <fw@strlen.de>
Date: 2017-02-06 23:01:52
Also in:
lkml
Cong Wang [off-list ref] wrote:
On Mon, Feb 6, 2017 at 11:39 AM, Andrey Konovalov [off-list ref] wrote:quoted
Hi, I've got the following error report while running the syzkaller fuzzer. The null-ptr-deref is caused by sendto() on a socket(PF_INET, SOCK_DGRAM, PROT_ICMP). Note, that this requires the ability to create such sockets, which can be configured by net.ipv4.ping_group_range (https://lwn.net/Articles/422330/). A reproducer and .config are attached. On commit a572a1b999489efb591287632279c6c9eca3e4ed. general protection fault: 0000 [#1] SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) Modules linked in: CPU: 2 PID: 3880 Comm: syz-executor1 Not tainted 4.10.0-rc6+ #124
[..]
quoted hunk ↗ jump to hunk
This fixes it for me:diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c index 86cca61..68d77b1 100644 --- a/net/ipv4/ping.c +++ b/net/ipv4/ping.c@@ -642,6 +642,8 @@ static int ping_v4_push_pending_frames(struct sock*sk, struct pingfakehdr *pfh, { struct sk_buff *skb = skb_peek(&sk->sk_write_queue); + if (!skb) + return 0; pfh->wcheck = csum_partial((char *)&pfh->icmph, sizeof(struct icmphdr), pfh->wcheck); pfh->icmph.checksum = csum_fold(pfh->wcheck);
Sigh. I wonder if we can remove ping sockets. IIRC they were born out of a 'no suid' requirement in combination with 'suid is insecure' but, alas, placing it in kernel evidently doesn't make things any more secure either. Those that don't want a suid ping binary could probably convince systemd developers to provide systemd-icmpd instead with ping dbus interface (ok, I'll shut up now ;)