From 3809a76e095d648d7d99cb1822ce17844a22ad2a Mon Sep 17 00:00:00 2001
From: Dexuan Cui <decui@microsoft.com>
Date: Fri, 5 May 2017 16:57:23 -0600
Subject: [PATCH 5/9] vmbus: dynamically enqueue/dequeue a channel on
 vmbus_open/close
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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@microsoft.com>
Reported-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Tested-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Origin: https://github.com/dcui/linux/commits/decui/hv_sock/v4.11/20170511
(cherry picked from commit 1df677b35ff010d0def33f5420773015815cf843)
---
 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 db5e6f8730d2..f288e506fba0 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 fbcb06352308..c5a01a4d589e 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 0c170a3f0d8b..ba93b7e4a972 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1437,6 +1437,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.12.2

