Commit a5d0e7c0 authored by Erik Hugne's avatar Erik Hugne Committed by David S. Miller

tipc: fix spinlock recursion bug for failed subscriptions

If a topology event subscription fails for any reason, such as out
of memory, max number reached or because we received an invalid
request the correct behavior is to terminate the subscribers
connection to the topology server. This is currently broken and
produces the following oops:

[27.953662] tipc: Subscription rejected, illegal request
[27.955329] BUG: spinlock recursion on CPU#1, kworker/u4:0/6
[27.957066]  lock: 0xffff88003c67f408, .magic: dead4ead, .owner: kworker/u4:0/6, .owner_cpu: 1
[27.958054] CPU: 1 PID: 6 Comm: kworker/u4:0 Not tainted 3.14.0-rc6+ #5
[27.960230] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[27.960874] Workqueue: tipc_rcv tipc_recv_work [tipc]
[27.961430]  ffff88003c67f408 ffff88003de27c18 ffffffff815c0207 ffff88003de1c050
[27.962292]  ffff88003de27c38 ffffffff815beec5 ffff88003c67f408 ffffffff817f0a8a
[27.963152]  ffff88003de27c58 ffffffff815beeeb ffff88003c67f408 ffffffffa0013520
[27.964023] Call Trace:
[27.964292]  [<ffffffff815c0207>] dump_stack+0x45/0x56
[27.964874]  [<ffffffff815beec5>] spin_dump+0x8c/0x91
[27.965420]  [<ffffffff815beeeb>] spin_bug+0x21/0x26
[27.965995]  [<ffffffff81083df6>] do_raw_spin_lock+0x116/0x140
[27.966631]  [<ffffffff815c6215>] _raw_spin_lock_bh+0x15/0x20
[27.967256]  [<ffffffffa0008540>] subscr_conn_shutdown_event+0x20/0xa0 [tipc]
[27.968051]  [<ffffffffa000fde4>] tipc_close_conn+0xa4/0xb0 [tipc]
[27.968722]  [<ffffffffa00101ba>] tipc_conn_terminate+0x1a/0x30 [tipc]
[27.969436]  [<ffffffffa00089a2>] subscr_conn_msg_event+0x1f2/0x2f0 [tipc]
[27.970209]  [<ffffffffa0010000>] tipc_receive_from_sock+0x90/0xf0 [tipc]
[27.970972]  [<ffffffffa000fa79>] tipc_recv_work+0x29/0x50 [tipc]
[27.971633]  [<ffffffff8105dbf5>] process_one_work+0x165/0x3e0
[27.972267]  [<ffffffff8105e869>] worker_thread+0x119/0x3a0
[27.972896]  [<ffffffff8105e750>] ? manage_workers.isra.25+0x2a0/0x2a0
[27.973622]  [<ffffffff810648af>] kthread+0xdf/0x100
[27.974168]  [<ffffffff810647d0>] ? kthread_create_on_node+0x1a0/0x1a0
[27.974893]  [<ffffffff815ce13c>] ret_from_fork+0x7c/0xb0
[27.975466]  [<ffffffff810647d0>] ? kthread_create_on_node+0x1a0/0x1a0

The recursion occurs when subscr_terminate tries to grab the
subscriber lock, which is already taken by subscr_conn_msg_event.
We fix this by checking if the request to establish a new
subscription was successful, and if not we initiate termination of
the subscriber after we have released the subscriber lock.
Signed-off-by: default avatarErik Hugne <erik.hugne@ericsson.com>
Reviewed-by: default avatarJon Maloy <jon.maloy@ericsson.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 4b29dba9
...@@ -263,9 +263,9 @@ static void subscr_cancel(struct tipc_subscr *s, ...@@ -263,9 +263,9 @@ static void subscr_cancel(struct tipc_subscr *s,
* *
* Called with subscriber lock held. * Called with subscriber lock held.
*/ */
static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s, static int subscr_subscribe(struct tipc_subscr *s,
struct tipc_subscriber *subscriber) struct tipc_subscriber *subscriber,
{ struct tipc_subscription **sub_p) {
struct tipc_subscription *sub; struct tipc_subscription *sub;
int swap; int swap;
...@@ -276,23 +276,21 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s, ...@@ -276,23 +276,21 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s,
if (s->filter & htohl(TIPC_SUB_CANCEL, swap)) { if (s->filter & htohl(TIPC_SUB_CANCEL, swap)) {
s->filter &= ~htohl(TIPC_SUB_CANCEL, swap); s->filter &= ~htohl(TIPC_SUB_CANCEL, swap);
subscr_cancel(s, subscriber); subscr_cancel(s, subscriber);
return NULL; return 0;
} }
/* Refuse subscription if global limit exceeded */ /* Refuse subscription if global limit exceeded */
if (atomic_read(&subscription_count) >= TIPC_MAX_SUBSCRIPTIONS) { if (atomic_read(&subscription_count) >= TIPC_MAX_SUBSCRIPTIONS) {
pr_warn("Subscription rejected, limit reached (%u)\n", pr_warn("Subscription rejected, limit reached (%u)\n",
TIPC_MAX_SUBSCRIPTIONS); TIPC_MAX_SUBSCRIPTIONS);
subscr_terminate(subscriber); return -EINVAL;
return NULL;
} }
/* Allocate subscription object */ /* Allocate subscription object */
sub = kmalloc(sizeof(*sub), GFP_ATOMIC); sub = kmalloc(sizeof(*sub), GFP_ATOMIC);
if (!sub) { if (!sub) {
pr_warn("Subscription rejected, no memory\n"); pr_warn("Subscription rejected, no memory\n");
subscr_terminate(subscriber); return -ENOMEM;
return NULL;
} }
/* Initialize subscription object */ /* Initialize subscription object */
...@@ -306,8 +304,7 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s, ...@@ -306,8 +304,7 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s,
(sub->seq.lower > sub->seq.upper)) { (sub->seq.lower > sub->seq.upper)) {
pr_warn("Subscription rejected, illegal request\n"); pr_warn("Subscription rejected, illegal request\n");
kfree(sub); kfree(sub);
subscr_terminate(subscriber); return -EINVAL;
return NULL;
} }
INIT_LIST_HEAD(&sub->nameseq_list); INIT_LIST_HEAD(&sub->nameseq_list);
list_add(&sub->subscription_list, &subscriber->subscription_list); list_add(&sub->subscription_list, &subscriber->subscription_list);
...@@ -320,8 +317,8 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s, ...@@ -320,8 +317,8 @@ static struct tipc_subscription *subscr_subscribe(struct tipc_subscr *s,
(Handler)subscr_timeout, (unsigned long)sub); (Handler)subscr_timeout, (unsigned long)sub);
k_start_timer(&sub->timer, sub->timeout); k_start_timer(&sub->timer, sub->timeout);
} }
*sub_p = sub;
return sub; return 0;
} }
/* Handle one termination request for the subscriber */ /* Handle one termination request for the subscriber */
...@@ -335,10 +332,14 @@ static void subscr_conn_msg_event(int conid, struct sockaddr_tipc *addr, ...@@ -335,10 +332,14 @@ static void subscr_conn_msg_event(int conid, struct sockaddr_tipc *addr,
void *usr_data, void *buf, size_t len) void *usr_data, void *buf, size_t len)
{ {
struct tipc_subscriber *subscriber = usr_data; struct tipc_subscriber *subscriber = usr_data;
struct tipc_subscription *sub; struct tipc_subscription *sub = NULL;
spin_lock_bh(&subscriber->lock); spin_lock_bh(&subscriber->lock);
sub = subscr_subscribe((struct tipc_subscr *)buf, subscriber); if (subscr_subscribe((struct tipc_subscr *)buf, subscriber, &sub) < 0) {
spin_unlock_bh(&subscriber->lock);
subscr_terminate(subscriber);
return;
}
if (sub) if (sub)
tipc_nametbl_subscribe(sub); tipc_nametbl_subscribe(sub);
spin_unlock_bh(&subscriber->lock); spin_unlock_bh(&subscriber->lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment