[azure][PATCH 04/14] UBUNTU: SAUCE: vmbus: dynamically enqueue/dequeue a channel on vmbus_open/close

[Marcelo Henrique Cerri]

From: Dexuan Cui <decui at microsoft.com>

BugLink: http://bugs.launchpad.net/bugs/1698425

A just-closed channel may have a pending interrupt, and later when a new
channel with the same channel ID is not being fully initialized, the
pending interrupt of the previous channel with the same channel ID can run
the channel callback on the new channel data structure, causing a crash
of NULL pointer dereferencing.

Normally it’s pretty hard to reproduce the race condition, but it can
indeed happen with specially-designed hv_sock stress test cases.

Signed-off-by: Dexuan Cui <decui at microsoft.com>
Reported-by: Rolf Neugebauer <rolf.neugebauer at docker.com>
Tested-by: Rolf Neugebauer <rolf.neugebauer at docker.com>
Cc: K. Y. Srinivasan <kys at microsoft.com>
Cc: Haiyang Zhang <haiyangz at microsoft.com>
Cc: Stephen Hemminger <sthemmin at microsoft.com>
Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
---
 drivers/hv/channel.c      | 12 +++++++++---
 drivers/hv/channel_mgmt.c | 50 +++++++++++++++++++++--------------------------
 include/linux/hyperv.h    |  3 +++
 3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index c9414393c9bc..9403599c2492 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -177,6 +177,8 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		      &vmbus_connection.chn_msg_list);
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
+	hv_percpu_channel_enq(newchannel);
+
 	ret = vmbus_post_msg(open_msg,
 			     sizeof(struct vmbus_channel_open_channel), true);
 
@@ -189,23 +191,25 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 
 	if (ret != 0) {
 		err = ret;
-		goto error_free_gpadl;
+		goto error_deq_channel;
 	}
 
 	if (newchannel->rescind) {
 		err = -ENODEV;
-		goto error_free_gpadl;
+		goto error_deq_channel;
 	}
 
 	if (open_info->response.open_result.status) {
 		err = -EAGAIN;
-		goto error_free_gpadl;
+		goto error_deq_channel;
 	}
 
 	newchannel->state = CHANNEL_OPENED_STATE;
 	kfree(open_info);
 	return 0;
 
+error_deq_channel:
+	hv_percpu_channel_deq(newchannel);
 error_free_gpadl:
 	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
 	kfree(open_info);
@@ -551,6 +555,8 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
 		goto out;
 	}
 
+	hv_percpu_channel_deq(channel);
+
 	channel->state = CHANNEL_OPEN_STATE;
 	channel->sc_creation_callback = NULL;
 	/* Stop callback and cancel the timer asap */
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 735f9363f2e4..f4dda5ecfa0b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -363,6 +363,17 @@ static void percpu_channel_enq(void *arg)
 	list_add_tail_rcu(&channel->percpu_list, &hv_cpu->chan_list);
 }
 
+void hv_percpu_channel_enq(struct vmbus_channel *channel)
+{
+	if (channel->target_cpu != get_cpu())
+		smp_call_function_single(channel->target_cpu,
+					 percpu_channel_enq, channel, true);
+	else
+		percpu_channel_enq(channel);
+
+	put_cpu();
+}
+
 static void percpu_channel_deq(void *arg)
 {
 	struct vmbus_channel *channel = arg;
@@ -370,6 +381,17 @@ static void percpu_channel_deq(void *arg)
 	list_del_rcu(&channel->percpu_list);
 }
 
+void hv_percpu_channel_deq(struct vmbus_channel *channel)
+{
+	if (channel->target_cpu != get_cpu())
+		smp_call_function_single(channel->target_cpu,
+					 percpu_channel_deq, channel, true);
+	else
+		percpu_channel_deq(channel);
+
+	put_cpu();
+}
+
 
 static void vmbus_release_relid(u32 relid)
 {
@@ -390,15 +412,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 	BUG_ON(!channel->rescind);
 	BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
 
-	if (channel->target_cpu != get_cpu()) {
-		put_cpu();
-		smp_call_function_single(channel->target_cpu,
-					 percpu_channel_deq, channel, true);
-	} else {
-		percpu_channel_deq(channel);
-		put_cpu();
-	}
-
 	if (channel->primary_channel == NULL) {
 		list_del(&channel->listentry);
 
@@ -491,16 +504,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 
 	init_vp_index(newchannel, dev_type);
 
-	if (newchannel->target_cpu != get_cpu()) {
-		put_cpu();
-		smp_call_function_single(newchannel->target_cpu,
-					 percpu_channel_enq,
-					 newchannel, true);
-	} else {
-		percpu_channel_enq(newchannel);
-		put_cpu();
-	}
-
 	/*
 	 * This state is used to indicate a successful open
 	 * so that when we do close the channel normally, we
@@ -549,15 +552,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	list_del(&newchannel->listentry);
 	mutex_unlock(&vmbus_connection.channel_mutex);
 
-	if (newchannel->target_cpu != get_cpu()) {
-		put_cpu();
-		smp_call_function_single(newchannel->target_cpu,
-					 percpu_channel_deq, newchannel, true);
-	} else {
-		percpu_channel_deq(newchannel);
-		put_cpu();
-	}
-
 	vmbus_release_relid(newchannel->offermsg.child_relid);
 
 err_free_chan:
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index e09fc8290c2f..b6975010e798 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1454,6 +1454,9 @@ extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf,
 				const int *srv_version, int srv_vercnt,
 				int *nego_fw_version, int *nego_srv_version);
 
+void hv_percpu_channel_enq(struct vmbus_channel *channel);
+void hv_percpu_channel_deq(struct vmbus_channel *channel);
+
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid);
 
 void vmbus_setevent(struct vmbus_channel *channel);
-- 
2.7.4