| Message ID | 1490398185.24891.5.camel@edumazet-glaptop3.roam.corp.google.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Mar 24, 2017 at 04:29:45PM -0700, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > We got a report of yet another bug in ping > > http://www.openwall.com/lists/oss-security/2017/03/24/6 > > ->disconnect() is not called with socket lock held. > > Fix this by acquiring ping rwlock earlier. > > Thanks to Alexander and Andrey for letting us know of this problem. > > Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind") > Reported-by: Solar Designer <solar@openwall.com> > Reported-by: Andrey Konovalov <andreyknvl@google.com> We should credit the original reporter, who most likely found this by fuzzing. It's danieljiang0415 on GitHub and Twitter. Unfortunately, I don't know their e-mail address. I'll try asking. Alexander
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 24 Mar 2017 16:29:45 -0700 > From: Eric Dumazet <edumazet@google.com> > > We got a report of yet another bug in ping > > http://www.openwall.com/lists/oss-security/2017/03/24/6 > > ->disconnect() is not called with socket lock held. > > Fix this by acquiring ping rwlock earlier. > > Thanks to Alexander and Andrey for letting us know of this problem. > > Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind") > Reported-by: Solar Designer <solar@openwall.com> > Reported-by: Andrey Konovalov <andreyknvl@google.com> Eric, please add a proper signoff for yourself, and also please add the following tag: Reported-by: Daniel Jiang <danieljiang0415@gmail.com> Thank you.
On Fri, Mar 24, 2017 at 7:10 PM, David Miller <davem@davemloft.net> wrote: > > > Eric, please add a proper signoff for yourself, and also please > add the following tag: > > Reported-by: Daniel Jiang <danieljiang0415@gmail.com> Oh right, will send a v2 immediately. Thanks.
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c index 2af6244b83e27ae384e96cf071c10c5a89674804..ccfbce13a6333a65dab64e4847dd510dfafb1b43 100644 --- a/net/ipv4/ping.c +++ b/net/ipv4/ping.c @@ -156,17 +156,18 @@ int ping_hash(struct sock *sk) void ping_unhash(struct sock *sk) { struct inet_sock *isk = inet_sk(sk); + pr_debug("ping_unhash(isk=%p,isk->num=%u)\n", isk, isk->inet_num); + write_lock_bh(&ping_table.lock); if (sk_hashed(sk)) { - write_lock_bh(&ping_table.lock); hlist_nulls_del(&sk->sk_nulls_node); sk_nulls_node_init(&sk->sk_nulls_node); sock_put(sk); isk->inet_num = 0; isk->inet_sport = 0; sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1); - write_unlock_bh(&ping_table.lock); } + write_unlock_bh(&ping_table.lock); } EXPORT_SYMBOL_GPL(ping_unhash);
From: Eric Dumazet <edumazet@google.com> We got a report of yet another bug in ping http://www.openwall.com/lists/oss-security/2017/03/24/6 ->disconnect() is not called with socket lock held. Fix this by acquiring ping rwlock earlier. Thanks to Alexander and Andrey for letting us know of this problem. Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind") Reported-by: Solar Designer <solar@openwall.com> Reported-by: Andrey Konovalov <andreyknvl@google.com> --- net/ipv4/ping.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)