Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Concurrent module unload and kexec execution causes a Time-of-Check to Time-of-Use (TOCTOU) Use-After-Free/NULL pointer dereference. Pre-existing issues: - [High] vmbus_initiate_unload() ignores vmbus_post_msg() errors, hanging kexec indefinitely on wait_for_completion(). -- --- Patch [2]: [RFC PATCH 2/2] arm64/hyperv: Add kexec handler using machine_shutdown hook --- commit 49c11bb6b5ac02d64fdd78fc6417342ccc8089ed Author: Shradha Gupta <[email protected]> arm64/hyperv: Add kexec handler using machine_shutdown hook This patch provides the ARM64 override of hv_setup_kexec_handler() and hv_remove_kexec_handler(). It wires the registered hv_kexec_handler to arm64_pre_smp_shutdown_hook to perform VMBus teardown and disable SynIC during kexec. > diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c > index 4fdc26ade1d74..4176175060d30 100644 > --- a/arch/arm64/hyperv/mshyperv.c > +++ b/arch/arm64/hyperv/mshyperv.c [ ... ] > @@ -15,10 +15,47 @@ > #include <linux/errno.h> > #include <linux/version.h> > #include <linux/cpuhotplug.h> > +#include <linux/kexec.h> > #include <asm/mshyperv.h> > +#include <asm/system_misc.h> > > static bool hyperv_initialized; > > +/* > + * Kexec/shutdown handler for ARM64 Hyper-V guests. > + * > + * On x86, hv_machine_shutdown() overrides machine_ops.shutdown and > + * runs after device_shutdown() and cpu_hotplug_enable(). ARM64 uses > + * the arm64_pre_smp_shutdown_hook to achieve the same ordering. > + * > + * hv_kexec_handler() (set by vmbus_drv.c) performs: > + * 1. vmbus_initiate_unload(false) - sends CHANNELMSG_UNLOAD > + * 2. cpuhp_remove_state(hyperv_cpuhp_online) - disables SynIC per CPU > + * > + * By running after device_shutdown(), PCI drivers (NVMe, MANA) can > + * send PCI_DELETE_INTERRUPT_MESSAGE and clean up MMIO/interrupt > + * mappings before VMBus channels are force-closed by UNLOAD. > + */ > +static void (*hv_kexec_handler)(void); > + > +static void hv_machine_shutdown(void) > +{ > + if (kexec_in_progress && hv_kexec_handler) > + hv_kexec_handler(); [Severity: High] Is there a race condition here if the hv_vmbus module is unloaded concurrently? If sys_delete_module() calls hv_remove_kexec_handler(), it clears hv_kexec_handler to NULL. If that happens after the check here but before execution, could this jump to a NULL pointer? Or if the module memory is freed before execution, could this cause a use-after-free panic? A simple READ_ONCE() would prevent a compiler double-fetch, but might not prevent the underlying use-after-free without proper synchronization like RCU or a mutex. [Severity: High] This is a pre-existing issue, but enabling this path on ARM64 exposes the system to a potential indefinite hang during kexec. When hv_kexec_handler() calls vmbus_initiate_unload(false), it ignores errors from vmbus_post_msg() and unconditionally waits for completion: drivers/hv/channel_mgmt.c:vmbus_initiate_unload() { ... vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header), !crash); /* * vmbus_initiate_unload() is also called on crash and the crash can be * happening in an interrupt context, where scheduling is impossible. */ if (!crash) wait_for_completion(&vmbus_connection.unload_event); ... } If vmbus_post_msg() fails (e.g., returning -ENOBUFS if the hypervisor is out of buffers), the unload message is never sent, the completion is never signaled, and the thread waits forever. Is there a way to handle this error instead of hanging the reboot process? > +} > + > +void hv_setup_kexec_handler(void (*handler)(void)) > +{ > + hv_kexec_handler = handler; > + arm64_pre_smp_shutdown_hook = hv_machine_shutdown; > +} > + > +void hv_remove_kexec_handler(void) > +{ > + arm64_pre_smp_shutdown_hook = NULL; > + hv_kexec_handler = NULL; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
