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 <[email protected]> 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/[email protected]/ > 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/[email protected]?part=1
