aboutsummaryrefslogtreecommitdiffstats
path: root/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 /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 'net')
-rw-r--r--net/sctp/input.c22
-rw-r--r--net/sctp/sm_sideeffect.c35
-rw-r--r--net/sctp/transport.c2
3 files changed, 55 insertions, 4 deletions
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 2a570184e5a9..ea2192444ce6 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -440,11 +440,25 @@ void sctp_icmp_proto_unreachable(struct sock *sk,
440{ 440{
441 SCTP_DEBUG_PRINTK("%s\n", __func__); 441 SCTP_DEBUG_PRINTK("%s\n", __func__);
442 442
443 sctp_do_sm(SCTP_EVENT_T_OTHER, 443 if (sock_owned_by_user(sk)) {
444 SCTP_ST_OTHER(SCTP_EVENT_ICMP_PROTO_UNREACH), 444 if (timer_pending(&t->proto_unreach_timer))
445 asoc->state, asoc->ep, asoc, t, 445 return;
446 GFP_ATOMIC); 446 else {
447 if (!mod_timer(&t->proto_unreach_timer,
448 jiffies + (HZ/20)))
449 sctp_association_hold(asoc);
450 }
451
452 } else {
453 if (timer_pending(&t->proto_unreach_timer) &&
454 del_timer(&t->proto_unreach_timer))
455 sctp_association_put(asoc);
447 456
457 sctp_do_sm(SCTP_EVENT_T_OTHER,
458 SCTP_ST_OTHER(SCTP_EVENT_ICMP_PROTO_UNREACH),
459 asoc->state, asoc->ep, asoc, t,
460 GFP_ATOMIC);
461 }
448} 462}
449 463
450/* Common lookup code for icmp/icmpv6 error handler. */ 464/* Common lookup code for icmp/icmpv6 error handler. */
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index d5ae450b6f02..eb1f42f45fdd 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -397,6 +397,41 @@ out_unlock:
397 sctp_transport_put(transport); 397 sctp_transport_put(transport);
398} 398}
399 399
400/* Handle the timeout of the ICMP protocol unreachable timer. Trigger
401 * the correct state machine transition that will close the association.
402 */
403void sctp_generate_proto_unreach_event(unsigned long data)
404{
405 struct sctp_transport *transport = (struct sctp_transport *) data;
406 struct sctp_association *asoc = transport->asoc;
407
408 sctp_bh_lock_sock(asoc->base.sk);
409 if (sock_owned_by_user(asoc->base.sk)) {
410 SCTP_DEBUG_PRINTK("%s:Sock is busy.\n", __func__);
411
412 /* Try again later. */
413 if (!mod_timer(&transport->proto_unreach_timer,
414 jiffies + (HZ/20)))
415 sctp_association_hold(asoc);
416 goto out_unlock;
417 }
418
419 /* Is this structure just waiting around for us to actually
420 * get destroyed?
421 */
422 if (asoc->base.dead)
423 goto out_unlock;
424
425 sctp_do_sm(SCTP_EVENT_T_OTHER,
426 SCTP_ST_OTHER(SCTP_EVENT_ICMP_PROTO_UNREACH),
427 asoc->state, asoc->ep, asoc, transport, GFP_ATOMIC);
428
429out_unlock:
430 sctp_bh_unlock_sock(asoc->base.sk);
431 sctp_association_put(asoc);
432}
433
434
400/* Inject a SACK Timeout event into the state machine. */ 435/* Inject a SACK Timeout event into the state machine. */
401static void sctp_generate_sack_event(unsigned long data) 436static void sctp_generate_sack_event(unsigned long data)
402{ 437{
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index be4d63d5a5cc..4a368038d46f 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -108,6 +108,8 @@ static struct sctp_transport *sctp_transport_init(struct sctp_transport *peer,
108 (unsigned long)peer); 108 (unsigned long)peer);
109 setup_timer(&peer->hb_timer, sctp_generate_heartbeat_event, 109 setup_timer(&peer->hb_timer, sctp_generate_heartbeat_event,
110 (unsigned long)peer); 110 (unsigned long)peer);
111 setup_timer(&peer->proto_unreach_timer,
112 sctp_generate_proto_unreach_event, (unsigned long)peer);
111 113
112 /* Initialize the 64-bit random nonce sent with heartbeat. */ 114 /* Initialize the 64-bit random nonce sent with heartbeat. */
113 get_random_bytes(&peer->hb_nonce, sizeof(peer->hb_nonce)); 115 get_random_bytes(&peer->hb_nonce, sizeof(peer->hb_nonce));