diff options
author | David Hildenbrand <david@redhat.com> | 2017-11-07 12:04:05 -0500 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2017-11-17 07:20:15 -0500 |
commit | 4d772cb85f64c16eca00177089ecb3cd5d292120 (patch) | |
tree | e1d42c7bcf88c9502cf35e118c26e50bbff634c9 | |
parent | 5af4157388adad82c339e3742fb6b67840721347 (diff) |
KVM: x86: fix em_fxstor() sleeping while in atomic
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>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
-rw-r--r-- | arch/x86/kvm/emulate.c | 32 |
1 files changed, 26 insertions, 6 deletions
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index b4a87debbb4b..e7d04d0c8008 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) | |||
4014 | fxstate_size(ctxt)); | 4014 | fxstate_size(ctxt)); |
4015 | } | 4015 | } |
4016 | 4016 | ||
4017 | /* | ||
4018 | * FXRSTOR might restore XMM registers not provided by the guest. Fill | ||
4019 | * in the host registers (via FXSAVE) instead, so they won't be modified. | ||
4020 | * (preemption has to stay disabled until FXRSTOR). | ||
4021 | * | ||
4022 | * Use noinline to keep the stack for other functions called by callers small. | ||
4023 | */ | ||
4024 | static noinline int fxregs_fixup(struct fxregs_state *fx_state, | ||
4025 | const size_t used_size) | ||
4026 | { | ||
4027 | struct fxregs_state fx_tmp; | ||
4028 | int rc; | ||
4029 | |||
4030 | rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_tmp)); | ||
4031 | memcpy((void *)fx_state + used_size, (void *)&fx_tmp + used_size, | ||
4032 | __fxstate_size(16) - used_size); | ||
4033 | |||
4034 | return rc; | ||
4035 | } | ||
4036 | |||
4017 | static int em_fxrstor(struct x86_emulate_ctxt *ctxt) | 4037 | static int em_fxrstor(struct x86_emulate_ctxt *ctxt) |
4018 | { | 4038 | { |
4019 | struct fxregs_state fx_state; | 4039 | struct fxregs_state fx_state; |
@@ -4024,19 +4044,19 @@ static int em_fxrstor(struct x86_emulate_ctxt *ctxt) | |||
4024 | if (rc != X86EMUL_CONTINUE) | 4044 | if (rc != X86EMUL_CONTINUE) |
4025 | return rc; | 4045 | return rc; |
4026 | 4046 | ||
4047 | size = fxstate_size(ctxt); | ||
4048 | rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, size); | ||
4049 | if (rc != X86EMUL_CONTINUE) | ||
4050 | return rc; | ||
4051 | |||
4027 | ctxt->ops->get_fpu(ctxt); | 4052 | ctxt->ops->get_fpu(ctxt); |
4028 | 4053 | ||
4029 | size = fxstate_size(ctxt); | ||
4030 | if (size < __fxstate_size(16)) { | 4054 | if (size < __fxstate_size(16)) { |
4031 | rc = asm_safe("fxsave %[fx]", , [fx] "+m"(fx_state)); | 4055 | rc = fxregs_fixup(&fx_state, size); |
4032 | if (rc != X86EMUL_CONTINUE) | 4056 | if (rc != X86EMUL_CONTINUE) |
4033 | goto out; | 4057 | goto out; |
4034 | } | 4058 | } |
4035 | 4059 | ||
4036 | rc = segmented_read_std(ctxt, ctxt->memop.addr.mem, &fx_state, size); | ||
4037 | if (rc != X86EMUL_CONTINUE) | ||
4038 | goto out; | ||
4039 | |||
4040 | if (fx_state.mxcsr >> 16) { | 4060 | if (fx_state.mxcsr >> 16) { |
4041 | rc = emulate_gp(ctxt, 0); | 4061 | rc = emulate_gp(ctxt, 0); |
4042 | goto out; | 4062 | goto out; |