public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ben Hutchings <ben@decadent.org.uk>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	Christoph Paasch <christoph.paasch@uclouvain.be>,
	"David S. Miller" <davem@davemloft.net>
Subject: [ 38/49] inet: Fix kmemleak in tcp_v4/6_syn_recv_sock and  dccp_v4/6_request_recv_sock
Date: Sun, 13 Jan 2013 17:43:33 +0000	[thread overview]
Message-ID: <20130113174303.752401921@decadent.org.uk> (raw)
In-Reply-To: <20130113174255.736888844@decadent.org.uk>

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Christoph Paasch <christoph.paasch@uclouvain.be>

[ Upstream commit e337e24d6624e74a558aa69071e112a65f7b5758 ]

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>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 include/net/inet_connection_sock.h |  1 +
 net/dccp/ipv4.c                    |  4 ++--
 net/dccp/ipv6.c                    |  3 ++-
 net/ipv4/inet_connection_sock.c    | 16 ++++++++++++++++
 net/ipv4/tcp_ipv4.c                |  5 ++---
 net/ipv6/tcp_ipv6.c                |  3 ++-
 6 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index e6db62e..ca2755f 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -317,6 +317,7 @@ extern void inet_csk_reqsk_queue_prune(struct sock *parent,
 				       const unsigned long max_rto);
 
 extern void inet_csk_destroy_sock(struct sock *sk);
+extern void inet_csk_prepare_forced_close(struct sock *sk);
 
 /*
  * LISTEN is a special case for poll..
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 3f4e541..72416c8 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -434,8 +434,8 @@ exit:
 	NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS);
 	return NULL;
 put_and_exit:
-	bh_unlock_sock(newsk);
-	sock_put(newsk);
+	inet_csk_prepare_forced_close(newsk);
+	dccp_done(newsk);
 	goto exit;
 }
 
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 17ee85c..592b78c 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -609,7 +609,8 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
 	newinet->inet_rcv_saddr = LOOPBACK4_IPV6;
 
 	if (__inet_inherit_port(sk, newsk) < 0) {
-		sock_put(newsk);
+		inet_csk_prepare_forced_close(newsk);
+		dccp_done(newsk);
 		goto out;
 	}
 	__inet6_hash(newsk, NULL);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index c14d88a..907ef2c 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -647,6 +647,22 @@ void inet_csk_destroy_sock(struct sock *sk)
 }
 EXPORT_SYMBOL(inet_csk_destroy_sock);
 
+/* This function allows to force a closure of a socket after the call to
+ * tcp/dccp_create_openreq_child().
+ */
+void inet_csk_prepare_forced_close(struct sock *sk)
+{
+	/* sk_clone_lock locked the socket and set refcnt to 2 */
+	bh_unlock_sock(sk);
+	sock_put(sk);
+
+	/* The below has to be done to allow calling inet_csk_destroy_sock */
+	sock_set_flag(sk, SOCK_DEAD);
+	percpu_counter_inc(sk->sk_prot->orphan_count);
+	inet_sk(sk)->inet_num = 0;
+}
+EXPORT_SYMBOL(inet_csk_prepare_forced_close);
+
 int inet_csk_listen_start(struct sock *sk, const int nr_table_entries)
 {
 	struct inet_sock *inet = inet_sk(sk);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 58c09a0..a97c9ad 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1520,9 +1520,8 @@ exit:
 	NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS);
 	return NULL;
 put_and_exit:
-	tcp_clear_xmit_timers(newsk);
-	bh_unlock_sock(newsk);
-	sock_put(newsk);
+	inet_csk_prepare_forced_close(newsk);
+	tcp_done(newsk);
 	goto exit;
 }
 EXPORT_SYMBOL(tcp_v4_syn_recv_sock);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index ccab3c8..db10805 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1524,7 +1524,8 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 #endif
 
 	if (__inet_inherit_port(sk, newsk) < 0) {
-		sock_put(newsk);
+		inet_csk_prepare_forced_close(newsk);
+		tcp_done(newsk);
 		goto out;
 	}
 	__inet6_hash(newsk, NULL);



  parent reply	other threads:[~2013-01-13 17:49 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-13 17:42 [ 00/49] 3.2.37-stable review Ben Hutchings
2013-01-13 17:42 ` [ 01/49] ext4: fix extent tree corruption caused by hole punch Ben Hutchings
2013-01-13 17:42 ` [ 02/49] i915: ensure that VGA plane is disabled Ben Hutchings
2013-01-13 17:42 ` [ 03/49] ext4: check dioread_nolock on remount Ben Hutchings
2013-01-13 17:42 ` [ 04/49] jbd2: fix assertion failure in jbd2_journal_flush() Ben Hutchings
2013-01-13 17:43 ` [ 05/49] hwmon: (lm73} Detect and report i2c bus errors Ben Hutchings
2013-01-13 17:43 ` [ 06/49] ext4: do not try to write superblock on ro remount w/o journal Ben Hutchings
2013-01-13 17:43 ` [ 07/49] PCI: Reduce Ricoh 0xe822 SD card reader base clock frequency to 50MHz Ben Hutchings
2013-01-13 17:43 ` [ 08/49] mm: Fix PageHead when !CONFIG_PAGEFLAGS_EXTENDED Ben Hutchings
2013-01-13 17:43 ` [ 09/49] cifs: adjust sequence number downward after signing NT_CANCEL request Ben Hutchings
2013-01-13 17:43 ` [ 10/49] tmpfs mempolicy: fix /proc/mounts corrupting memory Ben Hutchings
2013-01-13 17:43 ` [ 11/49] p54usb: add USB ID for T-Com Sinus 154 data II Ben Hutchings
2013-01-13 17:43 ` [ 12/49] ath9k_hw: Fix RX gain initvals for AR9485 Ben Hutchings
2013-01-13 17:43 ` [ 13/49] p54usb: add USBIDs for two more p54usb devices Ben Hutchings
2013-01-13 17:43 ` [ 14/49] powerpc/vdso: Remove redundant locking in update_vsyscall_tz() Ben Hutchings
2013-01-13 17:43 ` [ 15/49] powerpc: Add missing NULL terminator to avoid boot panic on PPC40x Ben Hutchings
2013-01-13 17:43 ` [ 16/49] drm/radeon: add connector table for SAM440ep embedded board Ben Hutchings
2013-01-13 17:43 ` [ 17/49] drm/radeon: add connector table for Mac G4 Silver Ben Hutchings
2013-01-14 18:48   ` Albrecht Dreß
2013-01-13 17:43 ` [ 18/49] drm/radeon: Properly handle DDC probe for DP bridges Ben Hutchings
2013-01-13 17:43 ` [ 19/49] NFSv4: Add ACCESS operation to OPEN compound Ben Hutchings
2013-01-15 18:42   ` Herton Ronaldo Krzesinski
2013-01-16  1:08     ` Ben Hutchings
2013-01-13 17:43 ` [ 20/49] NFSv4: dont check MAY_WRITE access bit in OPEN Ben Hutchings
2013-01-13 17:43 ` [ 21/49] NFS4: nfs4_opendata_access should return errno Ben Hutchings
2013-01-13 17:43 ` [ 22/49] NFS: Fix access to suid/sgid executables Ben Hutchings
2013-01-13 17:43 ` [ 23/49] drm/nouveau: fix init with agpgart-uninorth Ben Hutchings
2013-01-13 17:43 ` [ 24/49] video: mxsfb: fix crash when unblanking the display Ben Hutchings
2013-01-13 17:43 ` [ 25/49] nfs: fix null checking in nfs_get_option_str() Ben Hutchings
2013-01-13 17:43 ` [ 26/49] SUNRPC: Ensure that we free the rpc_task after cleanups are done Ben Hutchings
2013-01-13 17:43 ` [ 27/49] ACPI / scan: Do not use dummy HID for system bus ACPI nodes Ben Hutchings
2013-01-13 17:43 ` [ 28/49] mm: limit mmu_gather batching to fix soft lockups on !CONFIG_PREEMPT Ben Hutchings
2013-01-13 17:43 ` [ 29/49] drivers/rtc/rtc-vt8500.c: correct handling of CR_24H bitfield Ben Hutchings
2013-01-13 17:43 ` [ 30/49] drivers/rtc/rtc-vt8500.c: fix handling of data passed in struct rtc_time Ben Hutchings
2013-01-13 17:43 ` [ 31/49] udf: dont increment lenExtents while writing to a hole Ben Hutchings
2013-01-13 17:43 ` [ 32/49] epoll: prevent missed events on EPOLL_CTL_MOD Ben Hutchings
2013-01-13 17:43 ` [ 33/49] rt2x00: Dont let mac80211 send a BAR when an AMPDU subframe fails Ben Hutchings
2013-01-13 17:43 ` [ 34/49] mac80211: introduce IEEE80211_HW_TEARDOWN_AGGR_ON_BAR_FAIL Ben Hutchings
2013-01-13 17:43 ` [ 35/49] Revert: "rt2x00: Dont let mac80211 send a BAR when an AMPDU subframe fails" Ben Hutchings
2013-01-13 17:43 ` [ 36/49] ftrace: Do not function trace inlined functions Ben Hutchings
2013-01-13 17:43 ` [ 37/49] sparc: huge_ptep_set_* functions need to call set_huge_pte_at() Ben Hutchings
2013-01-13 17:43 ` Ben Hutchings [this message]
2013-01-13 17:43 ` [ 39/49] net: sched: integer overflow fix Ben Hutchings
2013-01-13 17:43 ` [ 40/49] tcp: implement RFC 5961 3.2 Ben Hutchings
2013-01-13 17:43 ` [ 41/49] tcp: implement RFC 5961 4.2 Ben Hutchings
2013-01-13 17:43 ` [ 42/49] tcp: refine SYN handling in tcp_validate_incoming Ben Hutchings
2013-01-13 17:43 ` [ 43/49] tcp: tcp_replace_ts_recent() should not be called from tcp_validate_incoming() Ben Hutchings
2013-01-13 17:43 ` [ 44/49] tcp: RFC 5961 5.2 Blind Data Injection Attack Mitigation Ben Hutchings
2013-01-13 17:43 ` [ 45/49] [SCSI] mvsas: Fix oops when ata commond timeout Ben Hutchings
2013-01-13 17:43 ` [ 46/49] RDMA/nes: Fix for crash when registering zero length MR for CQ Ben Hutchings
2013-01-13 17:43 ` [ 47/49] RDMA/nes: Fix for terminate timer crash Ben Hutchings
2013-01-13 17:43 ` [ 48/49] ACPI : do not use Lid and Sleep button for S5 wakeup Ben Hutchings
2013-01-13 17:43 ` [ 49/49] aoe: do not call bdi_init after blk_alloc_queue Ben Hutchings
2013-01-13 22:44 ` [ 00/49] 3.2.37-stable review Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130113174303.752401921@decadent.org.uk \
    --to=ben@decadent.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=christoph.paasch@uclouvain.be \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox