diff options
author | Christoph Paasch <christoph.paasch@uclouvain.be> | 2012-12-13 23:07:58 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2012-12-14 13:14:07 -0500 |
commit | e337e24d6624e74a558aa69071e112a65f7b5758 (patch) | |
tree | 5eddb5820d01310b93fd7466312fa02427c1c705 /net | |
parent | 093d04d42fa094f6740bb188f0ad0c215ff61e2c (diff) |
inet: Fix kmemleak in tcp_v4/6_syn_recv_sock and dccp_v4/6_request_recv_sock
If in either of the above functions inet_csk_route_child_sock() or
__inet_inherit_port() fails, the newsk will not be freed:
unreferenced object 0xffff88022e8a92c0 (size 1592):
comm "softirq", pid 0, jiffies 4294946244 (age 726.160s)
hex dump (first 32 bytes):
0a 01 01 01 0a 01 01 02 00 00 00 00 a7 cc 16 00 ................
02 00 03 01 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8153d190>] kmemleak_alloc+0x21/0x3e
[<ffffffff810ab3e7>] kmem_cache_alloc+0xb5/0xc5
[<ffffffff8149b65b>] sk_prot_alloc.isra.53+0x2b/0xcd
[<ffffffff8149b784>] sk_clone_lock+0x16/0x21e
[<ffffffff814d711a>] inet_csk_clone_lock+0x10/0x7b
[<ffffffff814ebbc3>] tcp_create_openreq_child+0x21/0x481
[<ffffffff814e8fa5>] tcp_v4_syn_recv_sock+0x3a/0x23b
[<ffffffff814ec5ba>] tcp_check_req+0x29f/0x416
[<ffffffff814e8e10>] tcp_v4_do_rcv+0x161/0x2bc
[<ffffffff814eb917>] tcp_v4_rcv+0x6c9/0x701
[<ffffffff814cea9f>] ip_local_deliver_finish+0x70/0xc4
[<ffffffff814cec20>] ip_local_deliver+0x4e/0x7f
[<ffffffff814ce9f8>] ip_rcv_finish+0x1fc/0x233
[<ffffffff814cee68>] ip_rcv+0x217/0x267
[<ffffffff814a7bbe>] __netif_receive_skb+0x49e/0x553
[<ffffffff814a7cc3>] netif_receive_skb+0x50/0x82
This happens, because sk_clone_lock initializes sk_refcnt to 2, and thus
a single sock_put() is not enough to free the memory. Additionally, things
like xfrm, memcg, cookie_values,... may have been initialized.
We have to free them properly.
This is fixed by forcing a call to tcp_done(), ending up in
inet_csk_destroy_sock, doing the final sock_put(). tcp_done() is necessary,
because it ends up doing all the cleanup on xfrm, memcg, cookie_values,
xfrm,...
Before calling tcp_done, we have to set the socket to SOCK_DEAD, to
force it entering inet_csk_destroy_sock. To avoid the warning in
inet_csk_destroy_sock, inet_num has to be set to 0.
As inet_csk_destroy_sock does a dec on orphan_count, we first have to
increase it.
Calling tcp_done() allows us to remove the calls to
tcp_clear_xmit_timer() and tcp_cleanup_congestion_control().
A similar approach is taken for dccp by calling dccp_done().
This is in the kernel since 093d282321 (tproxy: fix hash locking issue
when using port redirection in __inet_inherit_port()), thus since
version >= 2.6.37.
Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/dccp/ipv4.c | 4 | ||||
-rw-r--r-- | net/dccp/ipv6.c | 3 | ||||
-rw-r--r-- | net/ipv4/inet_connection_sock.c | 16 | ||||
-rw-r--r-- | net/ipv4/tcp_ipv4.c | 6 | ||||
-rw-r--r-- | net/ipv6/tcp_ipv6.c | 3 |
5 files changed, 24 insertions, 8 deletions
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c index 176ecdba4a22..4f9f5eb478f1 100644 --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c | |||
@@ -439,8 +439,8 @@ exit: | |||
439 | NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS); | 439 | NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS); |
440 | return NULL; | 440 | return NULL; |
441 | put_and_exit: | 441 | put_and_exit: |
442 | bh_unlock_sock(newsk); | 442 | inet_csk_prepare_forced_close(newsk); |
443 | sock_put(newsk); | 443 | dccp_done(newsk); |
444 | goto exit; | 444 | goto exit; |
445 | } | 445 | } |
446 | 446 | ||
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c index 56840b249f3b..6e05981f271e 100644 --- a/net/dccp/ipv6.c +++ b/net/dccp/ipv6.c | |||
@@ -585,7 +585,8 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk, | |||
585 | newinet->inet_rcv_saddr = LOOPBACK4_IPV6; | 585 | newinet->inet_rcv_saddr = LOOPBACK4_IPV6; |
586 | 586 | ||
587 | if (__inet_inherit_port(sk, newsk) < 0) { | 587 | if (__inet_inherit_port(sk, newsk) < 0) { |
588 | sock_put(newsk); | 588 | inet_csk_prepare_forced_close(newsk); |
589 | dccp_done(newsk); | ||
589 | goto out; | 590 | goto out; |
590 | } | 591 | } |
591 | __inet6_hash(newsk, NULL); | 592 | __inet6_hash(newsk, NULL); |
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 2026542d6836..d0670f00d524 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c | |||
@@ -710,6 +710,22 @@ void inet_csk_destroy_sock(struct sock *sk) | |||
710 | } | 710 | } |
711 | EXPORT_SYMBOL(inet_csk_destroy_sock); | 711 | EXPORT_SYMBOL(inet_csk_destroy_sock); |
712 | 712 | ||
713 | /* This function allows to force a closure of a socket after the call to | ||
714 | * tcp/dccp_create_openreq_child(). | ||
715 | */ | ||
716 | void inet_csk_prepare_forced_close(struct sock *sk) | ||
717 | { | ||
718 | /* sk_clone_lock locked the socket and set refcnt to 2 */ | ||
719 | bh_unlock_sock(sk); | ||
720 | sock_put(sk); | ||
721 | |||
722 | /* The below has to be done to allow calling inet_csk_destroy_sock */ | ||
723 | sock_set_flag(sk, SOCK_DEAD); | ||
724 | percpu_counter_inc(sk->sk_prot->orphan_count); | ||
725 | inet_sk(sk)->inet_num = 0; | ||
726 | } | ||
727 | EXPORT_SYMBOL(inet_csk_prepare_forced_close); | ||
728 | |||
713 | int inet_csk_listen_start(struct sock *sk, const int nr_table_entries) | 729 | int inet_csk_listen_start(struct sock *sk, const int nr_table_entries) |
714 | { | 730 | { |
715 | struct inet_sock *inet = inet_sk(sk); | 731 | struct inet_sock *inet = inet_sk(sk); |
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 1ed230716d51..54139fa514e6 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c | |||
@@ -1767,10 +1767,8 @@ exit: | |||
1767 | NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS); | 1767 | NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS); |
1768 | return NULL; | 1768 | return NULL; |
1769 | put_and_exit: | 1769 | put_and_exit: |
1770 | tcp_clear_xmit_timers(newsk); | 1770 | inet_csk_prepare_forced_close(newsk); |
1771 | tcp_cleanup_congestion_control(newsk); | 1771 | tcp_done(newsk); |
1772 | bh_unlock_sock(newsk); | ||
1773 | sock_put(newsk); | ||
1774 | goto exit; | 1772 | goto exit; |
1775 | } | 1773 | } |
1776 | EXPORT_SYMBOL(tcp_v4_syn_recv_sock); | 1774 | EXPORT_SYMBOL(tcp_v4_syn_recv_sock); |
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 6565cf55eb1e..93825dd3a7c0 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c | |||
@@ -1288,7 +1288,8 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb, | |||
1288 | #endif | 1288 | #endif |
1289 | 1289 | ||
1290 | if (__inet_inherit_port(sk, newsk) < 0) { | 1290 | if (__inet_inherit_port(sk, newsk) < 0) { |
1291 | sock_put(newsk); | 1291 | inet_csk_prepare_forced_close(newsk); |
1292 | tcp_done(newsk); | ||
1292 | goto out; | 1293 | goto out; |
1293 | } | 1294 | } |
1294 | __inet6_hash(newsk, NULL); | 1295 | __inet6_hash(newsk, NULL); |