Drivers: hv: vmbus: Fix bugs in rescind handling
Subsystems (2)
PATCH 1/1 Drivers: hv: vmbus: Fix bugs in rescind handling
From: "K. Y. Srinivasan" <[email protected]>
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 <[email protected]>
Reviewed-by Dexuan Cui <[email protected]>
Cc [email protected] (4.13 and above)
--- drivers/hv/channel.c | 6 +++--- drivers/hv/channel_mgmt.c | 37 ++++++++++++++++++------------------- drivers/hv/vmbus_drv.c | 3 +-- include/linux/hyperv.h | 2 +- 4 files changed, 23 insertions(+), 25 deletions(-)
drivers/hv/channel.c +3 -3
@@ -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);
drivers/hv/channel_mgmt.c +18 -19
@@ -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 ; }
@@ -843,7 +851,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;
@@ -882,16 +889,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. */
@@ -910,6 +907,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 ; }/*
@@ -918,6 +916,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); }
@@ -930,16 +929,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); } }
drivers/hv/vmbus_drv.c +1 -2
@@ -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);
include/linux/hyperv.h +1 -1
@@ -1439,7 +1439,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);/*
Signature · 1 line
2.14.1