netdev - Re: [PATCH v2] sctp: Fix a race between ICMP protocol unreachable and connect()

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]

Message-ID: <4BE775C7.9070702@cn.fujitsu.com>
Date:	Mon, 10 May 2010 10:56:07 +0800
From:	Wei Yongjun <yjwei@...fujitsu.com>
To:	Vlad Yasevich <vladislav.yasevich@...com>
CC:	davem@...emloft.net, netdev@...r.kernel.org,
	linux-sctp@...r.kernel.org
Subject: Re: [PATCH v2] sctp: Fix a race between ICMP protocol unreachable
 and connect()

Hi Vlad:

> [.. removed leftover debuggin printk. should probably be queued for stable
>  as well... ]
>
> ICMP protocol unreachable handling completely disregarded
> the fact that the user may have locket the socket.  It proceeded
> to destroy the association, even though the user may have
> held the lock and had a ref on the association.  This resulted
> in the following:
>
> Attempt to release alive inet socket f6afcc00
>
> =========================
> [ BUG: held lock freed! ]
> -------------------------
> somenu/2672 is freeing memory f6afcc00-f6afcfff, with a lock still held
> there!
>  (sk_lock-AF_INET){+.+.+.}, at: [<c122098a>] sctp_connect+0x13/0x4c
> 1 lock held by somenu/2672:
>  #0:  (sk_lock-AF_INET){+.+.+.}, at: [<c122098a>] sctp_connect+0x13/0x4c
>
> stack backtrace:
> Pid: 2672, comm: somenu Not tainted 2.6.32-telco #55
> Call Trace:
>  [<c1232266>] ? printk+0xf/0x11
>  [<c1038553>] debug_check_no_locks_freed+0xce/0xff
>  [<c10620b4>] kmem_cache_free+0x21/0x66
>  [<c1185f25>] __sk_free+0x9d/0xab
>  [<c1185f9c>] sk_free+0x1c/0x1e
>  [<c1216e38>] sctp_association_put+0x32/0x89
>  [<c1220865>] __sctp_connect+0x36d/0x3f4
>  [<c122098a>] ? sctp_connect+0x13/0x4c
>  [<c102d073>] ? autoremove_wake_function+0x0/0x33
>  [<c12209a8>] sctp_connect+0x31/0x4c
>  [<c11d1e80>] inet_dgram_connect+0x4b/0x55
>  [<c11834fa>] sys_connect+0x54/0x71
>  [<c103a3a2>] ? lock_release_non_nested+0x88/0x239
>  [<c1054026>] ? might_fault+0x42/0x7c
>  [<c1054026>] ? might_fault+0x42/0x7c
>  [<c11847ab>] sys_socketcall+0x6d/0x178
>  [<c10da994>] ? trace_hardirqs_on_thunk+0xc/0x10
>  [<c1002959>] syscall_call+0x7/0xb
>
> This was because the sctp_wait_for_connect() would aqcure the socket
> lock and then proceed to release the last reference count on the
> association, thus cause the fully destruction path to finish freeing
> the socket.
>
> The simplest solution is to start a very short timer in case the socket
> is owned by user.  When the timer expires, we can do some verification
> and be able to do the release properly.
>   

After reviewed this patch, I think we should delete active ICMP proto
unreachable timer when free transport. since I don't reproduce this BUG,
so I just do compile test only, sorry.

[PATCH] sctp: delete active ICMP proto unreachable timer when free transport

transport may be free before ICMP proto unreachable timer expire, so
we should delete active ICMP proto unreachable timer when transport
is going away.

Signed-off-by: Wei Yongjun <yjwei@...fujitsu.com>
---
 net/sctp/transport.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 4a36803..165d54e 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -173,6 +173,10 @@ void sctp_transport_free(struct sctp_transport *transport)
 	    del_timer(&transport->T3_rtx_timer))
 		sctp_transport_put(transport);
 
+	/* Delete the ICMP proto unreachable timer if it's active. */
+	if (timer_pending(&transport->proto_unreach_timer) &&
+	    del_timer(&transport->proto_unreach_timer))
+		sctp_association_put(transport->asoc);
 
 	sctp_transport_put(transport);
 }
-- 
1.6.5.2



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html