Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] The patch inadvertently leaves the BSP's kvmclock enabled when powering off the machine for ACPI S4 hibernation, leading to potential memory corruption when the VM awakens. Pre-existing issues: - [Critical] Stack-allocated waitqueue head use-after-free in async page fault handling. -- Note: The standard inline diff format has been altered to a free-form summary due to patch recitation restrictions. commit 03858970f2d7a88b91bdd5b16c1573166126d3c4 Author: Sean Christopherson <[email protected]> Subject: x86/kvm: Don't disable kvmclock on BSP in syscore_suspend() This patch modifies the guest suspend logic so that kvmclock is not disabled on the BSP during syscore_suspend(), relying instead on the sched_clock restore callback. [Severity: High] Does this change inadvertently leave the BSP's kvmclock enabled when powering off the machine for ACPI S4 hibernation? Looking at the final S4 power-off path in hibernation_platform_enter() in kernel/power/hibernate.c, it calls syscore_suspend() but does not call save_processor_state(). Because this patch removes the unconditional BSP kvmclock_disable() from syscore_suspend() and relies on kvm_save_sched_clock_state() (which only fires during image creation), kvmclock appears to remain fully enabled during the S4 power-off. Could this lead to memory corruption when the VM awakens, since the hypervisor might continue writing to the old kvmclock address? [Severity: Critical] This isn't a bug introduced by this patch, but is there a use-after-free risk with the stack-allocated waitqueue head in the async page fault handling in arch/x86/kernel/kvm.c? When a guest task waits for an async page fault in kvm_async_pf_task_wait_schedule(), it waits on a stack-allocated struct kvm_task_sleep_node. Concurrently, when a waker CPU executes apf_task_wake_one(), it unhashes the node via hlist_del_init(). The waiter checks hlist_unhashed() locklessly. If it observes the unhashed state before the waker is finished, the waiter can break its loop, return, and destroy the stack frame. The waker might then dereference the deallocated waitqueue head to call swake_up_one(), potentially corrupting stack memory. Can this sequence occur during concurrent page fault completions? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=29
