4.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dexuan Cui decui@microsoft.com
commit d3b26dd7cb0e3433bfd3c1d4dcf74c6039bb49fb upstream.
Before setting channel->rescind in vmbus_rescind_cleanup(), we should make sure the channel callback won't run any more, otherwise a high-level driver like pci_hyperv, which may be infinitely waiting for the host VSP's response and notices the channel has been rescinded, can't safely give up: e.g., in hv_pci_protocol_negotiation() -> wait_for_response(), it's unsafe to exit from wait_for_response() and proceed with the on-stack variable "comp_pkt" popped. The issue was originally spotted by Michael Kelley mikelley@microsoft.com.
In vmbus_close_internal(), the patch also minimizes the range protected by disabling/enabling channel->callback_event: we don't really need that for the whole function.
Signed-off-by: Dexuan Cui decui@microsoft.com Reviewed-by: Michael Kelley mikelley@microsoft.com Cc: stable@vger.kernel.org Cc: K. Y. Srinivasan kys@microsoft.com Cc: Stephen Hemminger sthemmin@microsoft.com Cc: Michael Kelley mikelley@microsoft.com Signed-off-by: K. Y. Srinivasan kys@microsoft.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/hv/channel.c | 40 ++++++++++++++++++++++++---------------- drivers/hv/channel_mgmt.c | 6 ++++++ include/linux/hyperv.h | 2 ++ 3 files changed, 32 insertions(+), 16 deletions(-)
--- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -558,11 +558,8 @@ static void reset_channel_cb(void *arg) channel->onchannel_callback = NULL; }
-static int vmbus_close_internal(struct vmbus_channel *channel) +void vmbus_reset_channel_cb(struct vmbus_channel *channel) { - struct vmbus_channel_close_channel *msg; - int ret; - /* * vmbus_on_event(), running in the per-channel tasklet, can race * with vmbus_close_internal() in the case of SMP guest, e.g., when @@ -572,6 +569,29 @@ static int vmbus_close_internal(struct v */ tasklet_disable(&channel->callback_event);
+ channel->sc_creation_callback = NULL; + + /* Stop the callback asap */ + if (channel->target_cpu != get_cpu()) { + put_cpu(); + smp_call_function_single(channel->target_cpu, reset_channel_cb, + channel, true); + } else { + reset_channel_cb(channel); + put_cpu(); + } + + /* Re-enable tasklet for use on re-open */ + tasklet_enable(&channel->callback_event); +} + +static int vmbus_close_internal(struct vmbus_channel *channel) +{ + struct vmbus_channel_close_channel *msg; + int ret; + + vmbus_reset_channel_cb(channel); + /* * In case a device driver's probe() fails (e.g., * util_probe() -> vmbus_open() returns -ENOMEM) and the device is @@ -585,16 +605,6 @@ static int vmbus_close_internal(struct v }
channel->state = CHANNEL_OPEN_STATE; - channel->sc_creation_callback = NULL; - /* Stop callback and cancel the timer asap */ - if (channel->target_cpu != get_cpu()) { - put_cpu(); - smp_call_function_single(channel->target_cpu, reset_channel_cb, - channel, true); - } else { - reset_channel_cb(channel); - put_cpu(); - }
/* Send a closing message */
@@ -639,8 +649,6 @@ static int vmbus_close_internal(struct v get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
out: - /* re-enable tasklet for use on re-open */ - tasklet_enable(&channel->callback_event); return ret; }
--- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -893,6 +893,12 @@ static void vmbus_onoffer_rescind(struct }
/* + * Before setting channel->rescind in vmbus_rescind_cleanup(), we + * should make sure the channel callback is not running any more. + */ + vmbus_reset_channel_cb(channel); + + /* * Now wait for offer handling to complete. */ vmbus_rescind_cleanup(channel); --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1046,6 +1046,8 @@ extern int vmbus_establish_gpadl(struct extern int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle);
+void vmbus_reset_channel_cb(struct vmbus_channel *channel); + extern int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer, u32 bufferlen,