| Message ID | 20240111231323.4043461-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/vmx: Multiple fixes | expand |
On 12.01.2024 00:13, Andrew Cooper wrote: > When receiving an INIT, a prior bugfix tried to ignore the INIT and continue > onwards. > > Unfortunately it's not safe to return at that point in vmx_vmexit_handler(). > Just out of context in the first hunk is a local_irqs_enabled() which is > depended-upon by the return-to-guest path, causing the following checklock > failure in debug builds: > > (XEN) Error: INIT received - ignoring > (XEN) CHECKLOCK FAILURE: prev irqsafe: 0, curr irqsafe 1 > (XEN) Xen BUG at common/spinlock.c:132 > (XEN) ----[ Xen-4.19-unstable x86_64 debug=y Tainted: H ]---- > ... > (XEN) Xen call trace: > (XEN) [<ffff82d040238e10>] R check_lock+0xcd/0xe1 > (XEN) [<ffff82d040238fe3>] F _spin_lock+0x1b/0x60 > (XEN) [<ffff82d0402ed6a8>] F pt_update_irq+0x32/0x3bb > (XEN) [<ffff82d0402b9632>] F vmx_intr_assist+0x3b/0x51d > (XEN) [<ffff82d040206447>] F vmx_asm_vmexit_handler+0xf7/0x210 > > Luckily, this is benign in release builds. Accidentally having IRQs disabled > when trying to take an IRQs-on lock isn't a deadlock-vulnerable pattern. > > Drop the problematic early return. In hindsight, it's wrong to skip other > normal VMExit steps. > > Fixes: b1f11273d5a7 ("x86/vmx: Don't spuriously crash the domain when INIT is received") > Reported-by: Reima ISHII <ishiir@g.ecc.u-tokyo.ac.jp> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 829c27f1fa33..a85394232a23 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -4090,7 +4090,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs) case EXIT_REASON_INIT: printk(XENLOG_ERR "Error: INIT received - ignoring\n"); - return; /* Renter the guest without further processing */ + break; } /* Now enable interrupts so it's safe to take locks. */ @@ -4692,6 +4692,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs) break; case EXIT_REASON_EXTERNAL_INTERRUPT: /* Handled earlier */ + case EXIT_REASON_INIT: /* Handled earlier */ case EXIT_REASON_TPR_BELOW_THRESHOLD: /* Handled later in vmx_intr_assist() */ case EXIT_REASON_BUS_LOCK: /* Nothing to do (rate-limit only) */ break;
When receiving an INIT, a prior bugfix tried to ignore the INIT and continue onwards. Unfortunately it's not safe to return at that point in vmx_vmexit_handler(). Just out of context in the first hunk is a local_irqs_enabled() which is depended-upon by the return-to-guest path, causing the following checklock failure in debug builds: (XEN) Error: INIT received - ignoring (XEN) CHECKLOCK FAILURE: prev irqsafe: 0, curr irqsafe 1 (XEN) Xen BUG at common/spinlock.c:132 (XEN) ----[ Xen-4.19-unstable x86_64 debug=y Tainted: H ]---- ... (XEN) Xen call trace: (XEN) [<ffff82d040238e10>] R check_lock+0xcd/0xe1 (XEN) [<ffff82d040238fe3>] F _spin_lock+0x1b/0x60 (XEN) [<ffff82d0402ed6a8>] F pt_update_irq+0x32/0x3bb (XEN) [<ffff82d0402b9632>] F vmx_intr_assist+0x3b/0x51d (XEN) [<ffff82d040206447>] F vmx_asm_vmexit_handler+0xf7/0x210 Luckily, this is benign in release builds. Accidentally having IRQs disabled when trying to take an IRQs-on lock isn't a deadlock-vulnerable pattern. Drop the problematic early return. In hindsight, it's wrong to skip other normal VMExit steps. Fixes: b1f11273d5a7 ("x86/vmx: Don't spuriously crash the domain when INIT is received") Reported-by: Reima ISHII <ishiir@g.ecc.u-tokyo.ac.jp> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> CC: Tamas K Lengyel <tamas@tklengyel.com> CC: Reima Ishii <ishiir@g.ecc.u-tokyo.ac.jp> CC: Takahiro Shinagawa <shina@ecc.u-tokyo.ac.jp> CC: George Dunlap <george.dunlap@citrix.com> With this patch in place, the INIT is ignored and the guest continues: (XEN) HVM1 restore: CPU 0 (d1) --- Xen Test Framework --- (d1) Environment: HVM 64bit (Long mode 4 levels) (XEN) Error: INIT received - ignoring (d1) Test result: SUCCESS v2: * Leave the printk() early, and skip in the main block. --- xen/arch/x86/hvm/vmx/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)