diff options
author | Vlad Yasevich <vladislav.yasevich@hp.com> | 2010-05-06 03:56:07 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2010-05-06 03:56:07 -0400 |
commit | 50b5d6ad63821cea324a5a7a19854d4de1a0a819 (patch) | |
tree | a279d53880e3bdf144783598ad03bbaa33e2cd96 /net | |
parent | 6ec82562ffc6f297d0de36d65776cff8e5704867 (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.c | 22 | ||||
-rw-r--r-- | net/sctp/sm_sideeffect.c | 35 | ||||
-rw-r--r-- | net/sctp/transport.c | 2 |
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 | */ | ||
403 | void 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 | |||
429 | out_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. */ |
401 | static void sctp_generate_sack_event(unsigned long data) | 436 | static 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)); |