Drivers: hv: vmbus: Fix bugs in rescind handling

This patch addresses the following bugs in the current rescind handling code:

1. Fixes a race condition where we may be invoking hv_process_channel_removal()
on an already freed channel.

2. Prevents indefinite wait when rescinding sub-channels by correctly setting
the probe_complete state.

I would like to thank Dexuan for patiently reviewing earlier versions of this
patch and identifying many of the issues fixed here.

Greg, please apply this to 4.14-final.

Fixes: '54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling")'

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Cc: stable@vger.kernel.org # (4.13 and above)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index efd5db743319282922a3fa9994bcee6c618781bb..894b67ac2cae509296cc6c421565df9b7ec48f7f 100644 (file)
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -640,6 +640,7 @@ void vmbus_close(struct vmbus_channel *channel)
                 */
                return;
        }
+       mutex_lock(&vmbus_connection.channel_mutex);
        /*
         * Close all the sub-channels first and then close the
         * primary channel.
@@ -648,16 +649,15 @@ void vmbus_close(struct vmbus_channel *channel)
                cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
                vmbus_close_internal(cur_channel);
                if (cur_channel->rescind) {
-                       mutex_lock(&vmbus_connection.channel_mutex);
-                       hv_process_channel_removal(cur_channel,
+                       hv_process_channel_removal(
                                           cur_channel->offermsg.child_relid);
-                       mutex_unlock(&vmbus_connection.channel_mutex);
                }
        }
        /*
         * Now close the primary.
         */
        vmbus_close_internal(channel);
+       mutex_unlock(&vmbus_connection.channel_mutex);
 }
 EXPORT_SYMBOL_GPL(vmbus_close);
 

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index bcbb031f726313937ecd7bcd6feb2c859f161588..018d2e0f8ec57b1a35c4bbc977e22e0228004d22 100644 (file)
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -159,7 +159,7 @@ static void vmbus_rescind_cleanup(struct vmbus_channel *channel)
 
 
        spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
-
+       channel->rescind = true;
        list_for_each_entry(msginfo, &vmbus_connection.chn_msg_list,
                                msglistentry) {
 
@@ -381,14 +381,21 @@ static void vmbus_release_relid(u32 relid)
                       true);
 }
 
-void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
+void hv_process_channel_removal(u32 relid)
 {
        unsigned long flags;
-       struct vmbus_channel *primary_channel;
+       struct vmbus_channel *primary_channel, *channel;
 
-       BUG_ON(!channel->rescind);
        BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
 
+       /*
+        * Make sure channel is valid as we may have raced.
+        */
+       channel = relid2channel(relid);
+       if (!channel)
+               return;
+
+       BUG_ON(!channel->rescind);
        if (channel->target_cpu != get_cpu()) {
                put_cpu();
                smp_call_function_single(channel->target_cpu,
@@ -515,6 +522,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
        if (!fnew) {
                if (channel->sc_creation_callback != NULL)
                        channel->sc_creation_callback(newchannel);
+               newchannel->probe_done = true;
                return;
        }
 
@@ -834,7 +842,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 {
        struct vmbus_channel_rescind_offer *rescind;
        struct vmbus_channel *channel;
-       unsigned long flags;
        struct device *dev;
 
        rescind = (struct vmbus_channel_rescind_offer *)hdr;
@@ -873,16 +880,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
                return;
        }
 
-       spin_lock_irqsave(&channel->lock, flags);
-       channel->rescind = true;
-       spin_unlock_irqrestore(&channel->lock, flags);
-
-       /*
-        * Now that we have posted the rescind state, perform
-        * rescind related cleanup.
-        */
-       vmbus_rescind_cleanup(channel);
-
        /*
         * Now wait for offer handling to complete.
         */
@@ -901,6 +898,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
        if (channel->device_obj) {
                if (channel->chn_rescind_callback) {
                        channel->chn_rescind_callback(channel);
+                       vmbus_rescind_cleanup(channel);
                        return;
                }
                /*
@@ -909,6 +907,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
                 */
                dev = get_device(&channel->device_obj->device);
                if (dev) {
+                       vmbus_rescind_cleanup(channel);
                        vmbus_device_unregister(channel->device_obj);
                        put_device(dev);
                }
@@ -921,16 +920,16 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
                 * 1. Close all sub-channels first
                 * 2. Then close the primary channel.
                 */
+               mutex_lock(&vmbus_connection.channel_mutex);
+               vmbus_rescind_cleanup(channel);
                if (channel->state == CHANNEL_OPEN_STATE) {
                        /*
                         * The channel is currently not open;
                         * it is safe for us to cleanup the channel.
                         */
-                       mutex_lock(&vmbus_connection.channel_mutex);
-                       hv_process_channel_removal(channel,
-                                               channel->offermsg.child_relid);
-                       mutex_unlock(&vmbus_connection.channel_mutex);
+                       hv_process_channel_removal(rescind->child_relid);
                }
+               mutex_unlock(&vmbus_connection.channel_mutex);
        }
 }
 

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index a9d49f6f6501ccb1965217abb429d13571f26f12..937801ac2fe0eafb3d148072a604c4e50a159572 100644 (file)
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -768,8 +768,7 @@ static void vmbus_device_release(struct device *device)
        struct vmbus_channel *channel = hv_dev->channel;
 
        mutex_lock(&vmbus_connection.channel_mutex);
-       hv_process_channel_removal(channel,
-                                  channel->offermsg.child_relid);
+       hv_process_channel_removal(channel->offermsg.child_relid);
        mutex_unlock(&vmbus_connection.channel_mutex);
        kfree(hv_dev);
 

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c458d7b7ad197688469772fb0c842180c668864c..6431087816ba5b06d9d5f7dae08af53ed57e11ca 100644 (file)
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1403,7 +1403,7 @@ 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_process_channel_removal(struct vmbus_channel *channel, u32 relid);
+void hv_process_channel_removal(u32 relid);
 
 void vmbus_setevent(struct vmbus_channel *channel);
 /*