| Message ID | 20171107170405.6989-1-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2017-11-07 18:04+0100, David Hildenbrand: > Commit 9d643f63128b ("KVM: x86: avoid large stack allocations in > em_fxrstor") optimize the stack size, but introduced a guest memory access > which might sleep while in atomic. > > Fix it by introducing, again, a second fxregs_state. Try to avoid > large stacks by using noinline. Add some helpful comments. > > Reported by syzbot: > > in_atomic(): 1, irqs_disabled(): 0, pid: 2909, name: syzkaller879109 > 2 locks held by syzkaller879109/2909: > #0: (&vcpu->mutex){+.+.}, at: [<ffffffff8106222c>] vcpu_load+0x1c/0x70 > arch/x86/kvm/../../../virt/kvm/kvm_main.c:154 > #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] vcpu_enter_guest > arch/x86/kvm/x86.c:6983 [inline] > #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] vcpu_run > arch/x86/kvm/x86.c:7061 [inline] > #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] > kvm_arch_vcpu_ioctl_run+0x1bc2/0x58b0 arch/x86/kvm/x86.c:7222 > CPU: 1 PID: 2909 Comm: syzkaller879109 Not tainted 4.13.0-rc4-next-20170811 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:16 [inline] > dump_stack+0x194/0x257 lib/dump_stack.c:52 > ___might_sleep+0x2b2/0x470 kernel/sched/core.c:6014 > __might_sleep+0x95/0x190 kernel/sched/core.c:5967 > __might_fault+0xab/0x1d0 mm/memory.c:4383 > __copy_from_user include/linux/uaccess.h:71 [inline] > __kvm_read_guest_page+0x58/0xa0 > arch/x86/kvm/../../../virt/kvm/kvm_main.c:1771 > kvm_vcpu_read_guest_page+0x44/0x60 > arch/x86/kvm/../../../virt/kvm/kvm_main.c:1791 > kvm_read_guest_virt_helper+0x76/0x140 arch/x86/kvm/x86.c:4407 > kvm_read_guest_virt_system+0x3c/0x50 arch/x86/kvm/x86.c:4466 > segmented_read_std+0x10c/0x180 arch/x86/kvm/emulate.c:819 > em_fxrstor+0x27b/0x410 arch/x86/kvm/emulate.c:4022 > x86_emulate_insn+0x55d/0x3c50 arch/x86/kvm/emulate.c:5471 > x86_emulate_instruction+0x411/0x1ca0 arch/x86/kvm/x86.c:5698 > kvm_mmu_page_fault+0x18b/0x2c0 arch/x86/kvm/mmu.c:4854 > handle_ept_violation+0x1fc/0x5e0 arch/x86/kvm/vmx.c:6400 > vmx_handle_exit+0x281/0x1ab0 arch/x86/kvm/vmx.c:8718 > vcpu_enter_guest arch/x86/kvm/x86.c:6999 [inline] > vcpu_run arch/x86/kvm/x86.c:7061 [inline] > kvm_arch_vcpu_ioctl_run+0x1cee/0x58b0 arch/x86/kvm/x86.c:7222 > kvm_vcpu_ioctl+0x64c/0x1010 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2591 > vfs_ioctl fs/ioctl.c:45 [inline] > do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:685 > SYSC_ioctl fs/ioctl.c:700 [inline] > SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691 > entry_SYSCALL_64_fastpath+0x1f/0xbe > RIP: 0033:0x437fc9 > RSP: 002b:00007ffc7b4d5ab8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010 > RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000437fc9 > RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 0000000000000005 > RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000020ae8000 > R10: 0000000000009120 R11: 0000000000000206 R12: 0000000000000000 > R13: 0000000000000004 R14: 0000000000000004 R15: 0000000020077000 > > Fixes: 9d643f63128b ("KVM: x86: avoid large stack allocations in em_fxrstor") > Signed-off-by: David Hildenbrand <david@redhat.com> > --- Applied, thanks.
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 8079d141792a..c4a603d8e9ad 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4014,6 +4014,26 @@ static int em_fxsave(struct x86_emulate_ctxt *ctxt) fxstate_size(ctxt)); } +/* + * FXRSTOR might restore XMM registers not provided by the guest. Fill + * in the host registers (via FXSAVE) instead, so they won't be modified. + * (preemption has to stay disabled until FXRSTOR). + * + * Use noinline to keep the stack for other functions called by callers small. + */ +static noinline int fxregs_fixup(struct fxregs_state *fx_state, + const size_t used_size) +{ + struct fxregs_state fx_tmp; + int rc; + + rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_tmp)); + memcpy((void *)fx_state + used_size, (void *)&fx_tmp + used_size, + __fxstate_size(16) - used_size); + + return rc; +} + static int em_fxrstor(struct x86_emulate_ctxt *ctxt) { struct fxregs_state fx_state; @@ -4024,19 +4044,19 @@ static int em_fxrstor(struct x86_emulate_ctxt *ctxt) if (rc != X86EMUL_CONTINUE) return rc; + size = fxstate_size(ctxt); + rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, size); + if (rc != X86EMUL_CONTINUE) + return rc; + ctxt->ops->get_fpu(ctxt); - size = fxstate_size(ctxt); if (size < __fxstate_size(16)) { - rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); + rc = fxregs_fixup(&fx_state, size); if (rc != X86EMUL_CONTINUE) goto out; } - rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, size); - if (rc != X86EMUL_CONTINUE) - goto out; - if (fx_state.mxcsr >> 16) { rc = emulate_gp(ctxt, 0); goto out;
Commit 9d643f63128b ("KVM: x86: avoid large stack allocations in em_fxrstor") optimize the stack size, but introduced a guest memory access which might sleep while in atomic. Fix it by introducing, again, a second fxregs_state. Try to avoid large stacks by using noinline. Add some helpful comments. Reported by syzbot: in_atomic(): 1, irqs_disabled(): 0, pid: 2909, name: syzkaller879109 2 locks held by syzkaller879109/2909: #0: (&vcpu->mutex){+.+.}, at: [<ffffffff8106222c>] vcpu_load+0x1c/0x70 arch/x86/kvm/../../../virt/kvm/kvm_main.c:154 #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] vcpu_enter_guest arch/x86/kvm/x86.c:6983 [inline] #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] vcpu_run arch/x86/kvm/x86.c:7061 [inline] #1: (&kvm->srcu){....}, at: [<ffffffff810dd162>] kvm_arch_vcpu_ioctl_run+0x1bc2/0x58b0 arch/x86/kvm/x86.c:7222 CPU: 1 PID: 2909 Comm: syzkaller879109 Not tainted 4.13.0-rc4-next-20170811 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 [inline] dump_stack+0x194/0x257 lib/dump_stack.c:52 ___might_sleep+0x2b2/0x470 kernel/sched/core.c:6014 __might_sleep+0x95/0x190 kernel/sched/core.c:5967 __might_fault+0xab/0x1d0 mm/memory.c:4383 __copy_from_user include/linux/uaccess.h:71 [inline] __kvm_read_guest_page+0x58/0xa0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1771 kvm_vcpu_read_guest_page+0x44/0x60 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1791 kvm_read_guest_virt_helper+0x76/0x140 arch/x86/kvm/x86.c:4407 kvm_read_guest_virt_system+0x3c/0x50 arch/x86/kvm/x86.c:4466 segmented_read_std+0x10c/0x180 arch/x86/kvm/emulate.c:819 em_fxrstor+0x27b/0x410 arch/x86/kvm/emulate.c:4022 x86_emulate_insn+0x55d/0x3c50 arch/x86/kvm/emulate.c:5471 x86_emulate_instruction+0x411/0x1ca0 arch/x86/kvm/x86.c:5698 kvm_mmu_page_fault+0x18b/0x2c0 arch/x86/kvm/mmu.c:4854 handle_ept_violation+0x1fc/0x5e0 arch/x86/kvm/vmx.c:6400 vmx_handle_exit+0x281/0x1ab0 arch/x86/kvm/vmx.c:8718 vcpu_enter_guest arch/x86/kvm/x86.c:6999 [inline] vcpu_run arch/x86/kvm/x86.c:7061 [inline] kvm_arch_vcpu_ioctl_run+0x1cee/0x58b0 arch/x86/kvm/x86.c:7222 kvm_vcpu_ioctl+0x64c/0x1010 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2591 vfs_ioctl fs/ioctl.c:45 [inline] do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:685 SYSC_ioctl fs/ioctl.c:700 [inline] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691 entry_SYSCALL_64_fastpath+0x1f/0xbe RIP: 0033:0x437fc9 RSP: 002b:00007ffc7b4d5ab8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000437fc9 RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 0000000000000005 RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000020ae8000 R10: 0000000000009120 R11: 0000000000000206 R12: 0000000000000000 R13: 0000000000000004 R14: 0000000000000004 R15: 0000000020077000 Fixes: 9d643f63128b ("KVM: x86: avoid large stack allocations in em_fxrstor") Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/x86/kvm/emulate.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)