[PATCH 1/1] Drivers: hv: Don't reset VMBus connection state on error/exit paths

[PATCH 1/1] Drivers: hv: Don't reset VMBus connection state on error/exit paths

[Michael Kelley]

In two places, current code forces vmbus_connection.conn_state to
DISCONNECTED prior to calling vmbus_disconnect(). But vmbus_disconnect()
calls vmbus_initiate_unload(), which unloads the VMBus connection
only if the conn_state is CONNECTED. Consequently, the connection
remains and the Hyper-V host might continue to send messages or post
VMBus interrupts.

The problem was introduced in commit 74347a99e73ae ("x86/Hyper-V:
Unload vmbus channel in hv panic callback"), in that
vmbus_initiate_unload() started checking the current connection
state before doing the unload. This commit removed some occurrences
where conn_state was forced to DISCONNECTED, but these two remained,
apparently due to an oversight at the time.

Fix this by not forcing conn_state to DISCONNECTED during VMBus exit,
or if a failure occurs when establishing the connection in the first
place. Whatever value conn_state has at the time is then read by
vmbus_initiate_unload() to determine if there is a connection to
be unloaded.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-hyperv/20260805205806.B5AC31F000E9@smtp.kernel.org/
Fixes: 74347a99e73ae ("x86/Hyper-V: Unload vmbus channel in hv panic callback")
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/hv/connection.c | 1 -
 drivers/hv/vmbus_drv.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 1ab3581b096a..818667ef9c74 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -346,7 +346,6 @@ int vmbus_connect(void)
 cleanup:
 	pr_err("Unable to connect to host\n");
 
-	vmbus_connection.conn_state = DISCONNECTED;
 	vmbus_disconnect();
 
 	kfree(msginfo);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 5ebdbe24b5a1..547bc731114f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -3043,7 +3043,6 @@ static void __exit vmbus_exit(void)
 
 	hv_remove_kexec_handler();
 	hv_remove_crash_handler();
-	vmbus_connection.conn_state = DISCONNECTED;
 	hv_stimer_global_cleanup();
 	vmbus_disconnect();
 	if (vmbus_irq == -1)
-- 
2.25.1

Re: [PATCH 1/1] Drivers: hv: Don't reset VMBus connection state on error/exit paths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return value from vmbus_post_msg() leading to an infinite wait during driver unload.
- [High] Destroying workqueues in vmbus_disconnect() before disabling VMBus interrupts leads to Use-After-Free if an asynchronous message is received.
--

commit bf55ca432bc98fbbbbd08f5b33b12b8f32d7705a
Author: Michael Kelley <mhkelley58@gmail.com>

Drivers: hv: Don't reset VMBus connection state on error/exit paths

This patch fixes an issue where the VMBus connection state was forced to
disconnected before calling vmbus_disconnect(). This prevented the
connection from being properly unloaded since vmbus_initiate_unload()
checks for the connected state. By leaving the connection state as is,
the hypervisor connection can be cleanly torn down.

Closes: https://lore.kernel.org/linux-hyperv/20260805205806.B5AC31F000E9@smtp.kernel.org/
Fixes: 74347a99e73ae ("x86/Hyper-V: Unload vmbus channel in hv panic callback")

[ ... ]

> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 5ebdbe24b5a1e..547bc731114fd 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -3043,7 +3043,6 @@ static void __exit vmbus_exit(void)
>  
>  	hv_remove_kexec_handler();
>  	hv_remove_crash_handler();
> -	vmbus_connection.conn_state = DISCONNECTED;
>  	hv_stimer_global_cleanup();
>  	vmbus_disconnect();

[Severity: High]
This isn't a bug introduced by this patch, but since this patch allows
vmbus_initiate_unload() to actively execute during vmbus_disconnect(),
does this expose a potential infinite wait?

Looking at vmbus_initiate_unload(), the code unconditionally waits for a
completion event even if the message posting fails:

drivers/hv/channel_mgmt.c:vmbus_initiate_unload() {
        ...
        vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header),
                       !crash);

        if (!crash)
                wait_for_completion(&vmbus_connection.unload_event);
}

If vmbus_post_msg() fails (for example, due to insufficient buffers),
the event will never be signaled by the host, causing the module unload
process to hang indefinitely. Could the return value of vmbus_post_msg()
be checked to prevent this?

[Severity: High]
This is also a pre-existing issue, but does the teardown ordering here
allow a use-after-free if an asynchronous message arrives during unload?

The current sequence calls vmbus_disconnect() which destroys the work
queues:

drivers/hv/connection.c:vmbus_disconnect() {
        ...
        if (vmbus_connection.work_queue)
                destroy_workqueue(vmbus_connection.work_queue);
        ...
}

However, VMBus interrupts and DPC tasklets are left active until later
in vmbus_exit():

drivers/hv/vmbus_drv.c:vmbus_exit() {
        ...
        vmbus_disconnect();

        if (vmbus_irq == -1)
                hv_remove_vmbus_handler();
        else
                free_percpu_irq(vmbus_irq, &vmbus_evt);
        ...
        for_each_online_cpu(cpu) {
                struct hv_per_cpu_context *hv_cpu = ...
                tasklet_kill(&hv_cpu->msg_dpc);
        }
}

If the hypervisor delivers an interrupt after vmbus_disconnect() returns
but before the DPC tasklets are killed, __vmbus_on_msg_dpc() can run and
attempt to queue work using the destroyed workqueue pointer:

drivers/hv/vmbus_drv.c:__vmbus_on_msg_dpc() {
        ...
        default:
                queue_work(vmbus_connection.work_queue, &ctx->work);
}

Should the interrupts and tasklets be disabled prior to freeing the
workqueues to avoid this race condition?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901034514.457089-1-mhklinux@outlook.com?part=1