aboutsummaryrefslogtreecommitdiffstats
path: root/include/net
diff options
context:
space:
mode:
authorVlad Yasevich <vladislav.yasevich@hp.com>2010-05-06 03:56:07 -0400
committerDavid S. Miller <davem@davemloft.net>2010-05-06 03:56:07 -0400
commit50b5d6ad63821cea324a5a7a19854d4de1a0a819 (patch)
treea279d53880e3bdf144783598ad03bbaa33e2cd96 /include/net
parent6ec82562ffc6f297d0de36d65776cff8e5704867 (diff)
sctp: Fix a race between ICMP protocol unreachable and connect()
ICMP protocol unreachable handling completely disregarded the fact that the user may have locked 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. Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'include/net')
-rw-r--r--include/net/sctp/sm.h1
-rw-r--r--include/net/sctp/structs.h3
2 files changed, 4 insertions, 0 deletions
diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index 851c813adb3a..61d73e37d543 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -279,6 +279,7 @@ int sctp_do_sm(sctp_event_t event_type, sctp_subtype_t subtype,
279/* 2nd level prototypes */ 279/* 2nd level prototypes */
280void sctp_generate_t3_rtx_event(unsigned long peer); 280void sctp_generate_t3_rtx_event(unsigned long peer);
281void sctp_generate_heartbeat_event(unsigned long peer); 281void sctp_generate_heartbeat_event(unsigned long peer);
282void sctp_generate_proto_unreach_event(unsigned long peer);
282 283
283void sctp_ootb_pkt_free(struct sctp_packet *); 284void sctp_ootb_pkt_free(struct sctp_packet *);
284 285
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 597f8e27aaf6..219043a67bf7 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1010,6 +1010,9 @@ struct sctp_transport {
1010 /* Heartbeat timer is per destination. */ 1010 /* Heartbeat timer is per destination. */
1011 struct timer_list hb_timer; 1011 struct timer_list hb_timer;
1012 1012
1013 /* Timer to handle ICMP proto unreachable envets */
1014 struct timer_list proto_unreach_timer;
1015
1013 /* Since we're using per-destination retransmission timers 1016 /* Since we're using per-destination retransmission timers
1014 * (see above), we're also using per-destination "transmitted" 1017 * (see above), we're also using per-destination "transmitted"
1015 * queues. This probably ought to be a private struct 1018 * queues. This probably ought to be a private struct