linux-kernel - Re: net/icmp: null-ptr-deref in ping_v4_push_pending

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:   Tue, 7 Feb 2017 00:00:41 +0100
From:   Florian Westphal <fw@...len.de>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Andrey Konovalov <andreyknvl@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        James Morris <jmorris@...ei.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Patrick McHardy <kaber@...sh.net>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Kostya Serebryany <kcc@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        syzkaller <syzkaller@...glegroups.com>
Subject: Re: net/icmp: null-ptr-deref in ping_v4_push_pending_frames

Cong Wang <xiyou.wangcong@...il.com> wrote:
> On Mon, Feb 6, 2017 at 11:39 AM, Andrey Konovalov <andreyknvl@...gle.com> wrote:
> > 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
[..]
> 
> 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 ;)