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>
This commit is contained in:
K. Y. Srinivasan 2017-09-29 21:09:36 -07:00 committed by Greg Kroah-Hartman
parent 688cb67839
commit 192b2d7872
4 changed files with 23 additions and 25 deletions

View File

@ -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);

View File

@ -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);
}
}

View File

@ -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);

View File

@ -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);
/*