* [PATCH net] ping: implement proper locking
@ 2017-03-24 23:29 Eric Dumazet
2017-03-25 0:30 ` Solar Designer
2017-03-25 2:10 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-03-24 23:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Solar Designer, Andrey Konovalov, Eric Dumazet
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(-)
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] ping: implement proper locking
2017-03-24 23:29 [PATCH net] ping: implement proper locking Eric Dumazet
@ 2017-03-25 0:30 ` Solar Designer
2017-03-25 2:10 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Solar Designer @ 2017-03-25 0:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Andrey Konovalov, Eric Dumazet
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] ping: implement proper locking
2017-03-24 23:29 [PATCH net] ping: implement proper locking Eric Dumazet
2017-03-25 0:30 ` Solar Designer
@ 2017-03-25 2:10 ` David Miller
2017-03-25 2:33 ` Eric Dumazet
2017-03-25 2:36 ` [PATCH v2 " Eric Dumazet
1 sibling, 2 replies; 6+ messages in thread
From: David Miller @ 2017-03-25 2:10 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, solar, andreyknvl, edumazet
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] ping: implement proper locking
2017-03-25 2:10 ` David Miller
@ 2017-03-25 2:33 ` Eric Dumazet
2017-03-25 2:36 ` [PATCH v2 " Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-03-25 2:33 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, netdev, solar, Andrey Konovalov
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 net] ping: implement proper locking
2017-03-25 2:10 ` David Miller
2017-03-25 2:33 ` Eric Dumazet
@ 2017-03-25 2:36 ` Eric Dumazet
2017-03-25 3:51 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2017-03-25 2:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev, solar, andreyknvl, edumazet, Daniel Jiang
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 Daniel, Alexander and Andrey for letting us know this problem.
Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Daniel Jiang <danieljiang0415@gmail.com>
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(-)
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 net] ping: implement proper locking
2017-03-25 2:36 ` [PATCH v2 " Eric Dumazet
@ 2017-03-25 3:51 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-03-25 3:51 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, solar, andreyknvl, edumazet, danieljiang0415
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 24 Mar 2017 19:36:13 -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 Daniel, Alexander and Andrey for letting us know this problem.
>
> Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Daniel Jiang <danieljiang0415@gmail.com>
> Reported-by: Solar Designer <solar@openwall.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-25 3:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-24 23:29 [PATCH net] ping: implement proper locking Eric Dumazet
2017-03-25 0:30 ` Solar Designer
2017-03-25 2:10 ` David Miller
2017-03-25 2:33 ` Eric Dumazet
2017-03-25 2:36 ` [PATCH v2 " Eric Dumazet
2017-03-25 3:51 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).