linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition
@ 2026-08-05 14:24 Michael Kelley
  2026-08-05 15:12 ` sashiko-bot
  2026-08-05 17:28 ` Easwar Hariharan
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Kelley @ 2026-08-05 14:24 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui, longli, linux-hyperv; +Cc: linux-kernel

The VMBus module initialization function, hv_acpi_init(), currently
does nothing when running in the root partition and root is not nested
in another VM. But the initialization function reports success, so the
VMBus module is indeed loaded. VMBus functionality is not actually
needed, but the VMBus module must be loaded so that hv_vmbus_exists()
can answer correctly. Furthermore, the mshv_root dependency on the
VMBus module is needed as described in the commit message for
840b740a35bf ("mshv: Add conditional VMBus dependency").

Loading the VMBus module without actually initializing it causes
failures if the module should later be unloaded. The module unload code
tries to clean up things that were never initialized, resulting in
memory faults and a panic.

Fix this by having VMBus module exit function perform the same
check for non-nested root partition, and do nothing in such a
case, just like hv_acpi_init().

In the long run, the code that manages the Hyper-V provided SynIC
should be refactored to better coordinate the requirements of
root partition scenarios and normal VM scenarios, and to hopefully
remove the hv_vmbus_exists() dependnecy between mshv_root and
VMBus modules. Preventing the current unload failure scenario is
an expediency until such a refactoring is done.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-hyperv/20260721154943.A09BD1F00A3D@smtp.kernel.org/
Fixes: 7e279d78664aa ("Drivers: hv: vmbus: skip VMBus initialization if Linux is root")
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
v1: https://lore.kernel.org/linux-hyperv/20260804190517.101981-1-mhklinux@outlook.com/

Changes in v2:
* Use a different solution: Allow the VMBus module to load but have the
  unload function do nothing for non-nested root
* Change the patch Subject and commit message to reflect the new approach

 drivers/hv/vmbus_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e19ec73b0187..f837153427f4 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -3024,6 +3024,9 @@ static void __exit vmbus_exit(void)
 {
 	int cpu;
 
+	if (hv_root_partition() && !hv_nested)
+		return;
+
 	unregister_syscore(&hv_synic_syscore);
 
 	hv_remove_kexec_handler();
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition
  2026-08-05 14:24 [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition Michael Kelley
@ 2026-08-05 15:12 ` sashiko-bot
  2026-08-05 17:28 ` Easwar Hariharan
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-05 15:12 UTC (permalink / raw)
  To: Michael Kelley; +Cc: linux-hyperv

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

Pre-existing issues:
- [High] Backwards teardown in `vmbus_exit()` causes failure to free IRQ action, leading to reload failure and potential Use-After-Free of unmapped IRQ handler.
--

--- Patch [1]: [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition ---
commit d186708fbf26f95dce2f00a8fed8b7ac79496439
Author: Michael Kelley <mhkelley58@gmail.com>

Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition

This commit updates the VMBus module exit function to skip cleanup when
running in a non-nested root partition, matching the behavior of the
initialization function. This prevents memory faults and kernel panics
caused by attempting to clean up uninitialized resources during module
unload.

Closes: https://lore.kernel.org/linux-hyperv/20260721154943.A09BD1F00A3D@smtp.kernel.org/

> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 6824bd7cb3c4f..88fc6add26860 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -3030,6 +3030,9 @@ static void __exit vmbus_exit(void)
>  {
>  	int cpu;
>  
> +	if (hv_root_partition() && !hv_nested)
> +		return;
> +
>  	unregister_syscore(&hv_synic_syscore);
>  
>  	hv_remove_kexec_handler();

[ ... ]

>  	vmbus_disconnect();
>  	if (vmbus_irq == -1)
>  		hv_remove_vmbus_handler();
>  	else
>  		free_percpu_irq(vmbus_irq, &vmbus_evt);

[Severity: High]
This is a pre-existing issue, but does calling free_percpu_irq() here abort 
and leak the irqaction?

It appears the CPUs have not yet disabled the interrupt, as 
cpuhp_remove_state() is called much later in this function. Because 
cpuhp_remove_state() is what triggers hv_synic_cleanup() to disable the 
percpu IRQ on each CPU, the IRQ is still locally enabled here.

This sequence typically trips the structural safety check in 
__free_percpu_irq() causing the free operation to abort and return NULL. 
If the IRQ line fires after the module is unloaded, could the kernel execute 
an unmapped function pointer?

[ ... ]

>  	bus_unregister(&hv_bus);
>  
>  	cpuhp_remove_state(hyperv_cpuhp_online);

[Severity: High]
This is related to the pre-existing issue above. Is this execution of 
cpuhp_remove_state() too late in the teardown sequence?

This step triggers the offline callback hv_synic_cleanup() which calls 
disable_percpu_irq(vmbus_irq). Because it runs after free_percpu_irq(), 
the IRQ action is permanently leaked and subsequent attempts to reload the 
module might fail.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition
  2026-08-05 14:24 [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition Michael Kelley
  2026-08-05 15:12 ` sashiko-bot
@ 2026-08-05 17:28 ` Easwar Hariharan
  1 sibling, 0 replies; 3+ messages in thread
From: Easwar Hariharan @ 2026-08-05 17:28 UTC (permalink / raw)
  To: mhklinux
  Cc: kys, haiyangz, wei.liu, decui, longli, linux-hyperv,
	easwar.hariharan, linux-kernel

On 8/5/2026 07:24, Michael Kelley wrote:
> The VMBus module initialization function, hv_acpi_init(), currently
> does nothing when running in the root partition and root is not nested
> in another VM. But the initialization function reports success, so the
> VMBus module is indeed loaded. VMBus functionality is not actually
> needed, but the VMBus module must be loaded so that hv_vmbus_exists()
> can answer correctly. Furthermore, the mshv_root dependency on the
> VMBus module is needed as described in the commit message for
> 840b740a35bf ("mshv: Add conditional VMBus dependency").
> 
> Loading the VMBus module without actually initializing it causes
> failures if the module should later be unloaded. The module unload code
> tries to clean up things that were never initialized, resulting in
> memory faults and a panic.
> 
> Fix this by having VMBus module exit function perform the same
> check for non-nested root partition, and do nothing in such a
> case, just like hv_acpi_init().
> 
> In the long run, the code that manages the Hyper-V provided SynIC
> should be refactored to better coordinate the requirements of
> root partition scenarios and normal VM scenarios, and to hopefully
> remove the hv_vmbus_exists() dependnecy between mshv_root and
> VMBus modules. Preventing the current unload failure scenario is
> an expediency until such a refactoring is done.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/linux-hyperv/20260721154943.A09BD1F00A3D@smtp.kernel.org/
> Fixes: 7e279d78664aa ("Drivers: hv: vmbus: skip VMBus initialization if Linux is root")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
> v1: https://lore.kernel.org/linux-hyperv/20260804190517.101981-1-mhklinux@outlook.com/
> 
> Changes in v2:
> * Use a different solution: Allow the VMBus module to load but have the
>   unload function do nothing for non-nested root
> * Change the patch Subject and commit message to reflect the new approach
> 
>  drivers/hv/vmbus_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index e19ec73b0187..f837153427f4 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -3024,6 +3024,9 @@ static void __exit vmbus_exit(void)
>  {
>  	int cpu;
>  
> +	if (hv_root_partition() && !hv_nested)
> +		return;
> +

Do you think it'd be useful to have a comment here either saying something to the effect of
"nothing was initialized, so let's skip the teardown", or alternatively pointing to the paired
check in hv_acpi_init()?

Either way,

Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>

>  	unregister_syscore(&hv_synic_syscore);
>  
>  	hv_remove_kexec_handler();


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-05 17:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 14:24 [PATCH v2 1/1] Drivers: hv: vmbus: Skip VMBus module cleanup for non-nested root partition Michael Kelley
2026-08-05 15:12 ` sashiko-bot
2026-08-05 17:28 ` Easwar Hariharan

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).