* FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind handling" failed to apply to 4.13-stable tree
@ 2017-10-15 13:28 gregkh
2017-10-16 22:00 ` Dexuan Cui
0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2017-10-15 13:28 UTC (permalink / raw)
To: kys, decui, gregkh; +Cc: stable
The patch below does not apply to the 4.13-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From 192b2d78722ffea188e5ec6ae5d55010dce05a4b Mon Sep 17 00:00:00 2001
From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Fri, 29 Sep 2017 21:09:36 -0700
Subject: [PATCH] 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 efd5db743319..894b67ac2cae 100644
--- 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 bcbb031f7263..018d2e0f8ec5 100644
--- 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 a9d49f6f6501..937801ac2fe0 100644
--- 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 c458d7b7ad19..6431087816ba 100644
--- 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);
/*
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind handling" failed to apply to 4.13-stable tree
2017-10-15 13:28 FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind handling" failed to apply to 4.13-stable tree gregkh
@ 2017-10-16 22:00 ` Dexuan Cui
2017-10-19 9:39 ` gregkh
0 siblings, 1 reply; 3+ messages in thread
From: Dexuan Cui @ 2017-10-16 22:00 UTC (permalink / raw)
To: gregkh@linuxfoundation.org, KY Srinivasan; +Cc: stable@vger.kernel.org
> From: gregkh@linuxfoundation.org [mailto:gregkh@linuxfoundation.org]
> Sent: Sunday, October 15, 2017 6:28 AM
> To: KY Srinivasan <kys@microsoft.com>; Dexuan Cui <decui@microsoft.com>;
> gregkh@linuxfoundation.org
> Cc: stable@vger.kernel.org
> Subject: FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind
> handling" failed to apply to 4.13-stable tree
>
> The patch below does not apply to the 4.13-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> thanks,
>
> greg k-h
Hi Greg,
Before applying this patch (192b2d7872), please cherry-pick the prerequisite
patch (6f3d791f30) in Linus's tree.
As I tested just now, the 2 patches applied cleanly on linux-4.13.y, which
Means v4.13.7 today.
Thanks,
-- Dexuan
commit 6f3d791f300618caf82a2be0c27456edd76d5164
Author: K. Y. Srinivasan <kys@microsoft.com>
Date: Fri Aug 11 10:03:59 2017 -0700
Drivers: hv: vmbus: Fix rescind handling issues
This patch handles the following issues that were observed when we are
handling racing channel offer message and rescind message for the same
offer:
1. Since the host does not respond to messages on a rescinded channel,
in the current code, we could be indefinitely blocked on the vmbus_open() call.
2. When a rescinded channel is being closed, if there is a pending interrupt on the
channel, we could end up freeing the channel that the interrupt handler would run on.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ------------------ original commit in Linus's tree ------------------
>
> From 192b2d78722ffea188e5ec6ae5d55010dce05a4b Mon Sep 17 00:00:00
> 2001
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Fri, 29 Sep 2017 21:09:36 -0700
> Subject: [PATCH] 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 efd5db743319..894b67ac2cae 100644
> --- 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 bcbb031f7263..018d2e0f8ec5 100644
> --- 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 a9d49f6f6501..937801ac2fe0 100644
> --- 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 c458d7b7ad19..6431087816ba 100644
> --- 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);
> /*
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind handling" failed to apply to 4.13-stable tree
2017-10-16 22:00 ` Dexuan Cui
@ 2017-10-19 9:39 ` gregkh
0 siblings, 0 replies; 3+ messages in thread
From: gregkh @ 2017-10-19 9:39 UTC (permalink / raw)
To: Dexuan Cui; +Cc: KY Srinivasan, stable@vger.kernel.org
On Mon, Oct 16, 2017 at 10:00:41PM +0000, Dexuan Cui wrote:
> > From: gregkh@linuxfoundation.org [mailto:gregkh@linuxfoundation.org]
> > Sent: Sunday, October 15, 2017 6:28 AM
> > To: KY Srinivasan <kys@microsoft.com>; Dexuan Cui <decui@microsoft.com>;
> > gregkh@linuxfoundation.org
> > Cc: stable@vger.kernel.org
> > Subject: FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind
> > handling" failed to apply to 4.13-stable tree
> >
> > The patch below does not apply to the 4.13-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg,
> Before applying this patch (192b2d7872), please cherry-pick the prerequisite
> patch (6f3d791f30) in Linus's tree.
>
> As I tested just now, the 2 patches applied cleanly on linux-4.13.y, which
> Means v4.13.7 today.
Ah, that worked, thanks!
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-19 9:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-15 13:28 FAILED: patch "[PATCH] Drivers: hv: vmbus: Fix bugs in rescind handling" failed to apply to 4.13-stable tree gregkh
2017-10-16 22:00 ` Dexuan Cui
2017-10-19 9:39 ` gregkh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).