| Message ID | 1355400716-21359-1-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 13, 2012 at 01:11:55PM +0100, Paolo Bonzini wrote: > MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > Some hypervisor implementations assumed the operand is 32-bit. This > should never happen because the instruction has no memory operand, but > (like the existing test_mmx_movq_mf) the testcase tricks the emulator > into executing one by mismatching the page tables and the corresponding > TLB entry. > BTW how the bug was found? Why instruction was emulated at all? May be there is bug somewhere that makes KVM emulate something it should not. > Cc: Nadav Amit <nadav.amit@gmail.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > x86/emulator.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/x86/emulator.c b/x86/emulator.c > index 969944a..d578347 100644 > --- a/x86/emulator.c > +++ b/x86/emulator.c > @@ -694,6 +694,38 @@ static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page, > handle_exception(MF_VECTOR, 0); > } > > +static void test_movabs(uint64_t *mem, uint8_t *insn_page, > + uint8_t *alt_insn_page, void *insn_ram) > +{ > + uint64_t val = 0; > + ulong *cr3 = (ulong *)read_cr3(); > + > + // Pad with RET instructions > + memset(insn_page, 0xc3, 4096); > + memset(alt_insn_page, 0xc3, 4096); > + // Place a trapping instruction in the page to trigger a VMEXIT > + insn_page[0] = 0x89; // mov %eax, (%rax) > + insn_page[1] = 0x00; > + // Place the instruction we want the hypervisor to see in the alternate > + // page. A buggy hypervisor will fetch a 32-bit immediate and return > + // 0xffffffffc3c3c3c3. > + alt_insn_page[0] = 0x48; // mov $0xc3c3c3c3c3c3c3c3, %rcx > + alt_insn_page[1] = 0xb9; > + > + // Load the code TLB with insn_page, but point the page tables at > + // alt_insn_page (and keep the data TLB clear, for AMD decode assist). > + // This will make the CPU trap on the insn_page instruction but the > + // hypervisor will see alt_insn_page. > + install_page(cr3, virt_to_phys(insn_page), insn_ram); > + // Load code TLB > + invlpg(insn_ram); > + asm volatile("call *%0" : : "r"(insn_ram + 3)); > + // Trap, let hypervisor emulate at alt_insn_page > + install_page(cr3, virt_to_phys(alt_insn_page), insn_ram); > + asm volatile("call *%1" : "=c"(val) : "r"(insn_ram), "a"(mem), "c"(0)); > + report("64-bit mov imm", val == 0xc3c3c3c3c3c3c3c3); > +} > + > static void test_crosspage_mmio(volatile uint8_t *mem) > { > volatile uint16_t w, *pw; > @@ -759,6 +791,7 @@ int main() > test_shld_shrd(mem); > > test_mmx_movq_mf(mem, insn_page, alt_insn_page, insn_ram); > + test_movabs(mem, insn_page, alt_insn_page, insn_ram); > > test_crosspage_mmio(mem); > > -- > 1.8.0.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 13, 2012 at 02:24:18PM +0200, Gleb Natapov wrote: > On Thu, Dec 13, 2012 at 01:11:55PM +0100, Paolo Bonzini wrote: > > MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > > Some hypervisor implementations assumed the operand is 32-bit. This > > should never happen because the instruction has no memory operand, but > > (like the existing test_mmx_movq_mf) the testcase tricks the emulator > > into executing one by mismatching the page tables and the corresponding > > TLB entry. > > > BTW how the bug was found? Why instruction was emulated at all? May be > there is bug somewhere that makes KVM emulate something it should not. During switch to protected mode. SS.DPL=3, SS.RPL=0. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 13, 2012 at 06:45:26PM -0200, Marcelo Tosatti wrote: > On Thu, Dec 13, 2012 at 02:24:18PM +0200, Gleb Natapov wrote: > > On Thu, Dec 13, 2012 at 01:11:55PM +0100, Paolo Bonzini wrote: > > > MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > > > Some hypervisor implementations assumed the operand is 32-bit. This > > > should never happen because the instruction has no memory operand, but > > > (like the existing test_mmx_movq_mf) the testcase tricks the emulator > > > into executing one by mismatching the page tables and the corresponding > > > TLB entry. > > > > > BTW how the bug was found? Why instruction was emulated at all? May be > > there is bug somewhere that makes KVM emulate something it should not. > > During switch to protected mode. SS.DPL=3, SS.RPL=0. Yes, looks like a bug. We set SS.DPL to 3 to enter vm86 and this leaks to protected mode. There are a lot of those. I am trying to fix this mess. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 13, 2012 at 01:11:55PM +0100, Paolo Bonzini wrote: > MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. > Some hypervisor implementations assumed the operand is 32-bit. This > should never happen because the instruction has no memory operand, but > (like the existing test_mmx_movq_mf) the testcase tricks the emulator > into executing one by mismatching the page tables and the corresponding > TLB entry. > > Cc: Nadav Amit <nadav.amit@gmail.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > x86/emulator.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) Wicked, missed that. If you're unlucky, though, TLB entry can be gone by then (unlikely, though). Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/x86/emulator.c b/x86/emulator.c index 969944a..d578347 100644 --- a/x86/emulator.c +++ b/x86/emulator.c @@ -694,6 +694,38 @@ static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page, handle_exception(MF_VECTOR, 0); } +static void test_movabs(uint64_t *mem, uint8_t *insn_page, + uint8_t *alt_insn_page, void *insn_ram) +{ + uint64_t val = 0; + ulong *cr3 = (ulong *)read_cr3(); + + // Pad with RET instructions + memset(insn_page, 0xc3, 4096); + memset(alt_insn_page, 0xc3, 4096); + // Place a trapping instruction in the page to trigger a VMEXIT + insn_page[0] = 0x89; // mov %eax, (%rax) + insn_page[1] = 0x00; + // Place the instruction we want the hypervisor to see in the alternate + // page. A buggy hypervisor will fetch a 32-bit immediate and return + // 0xffffffffc3c3c3c3. + alt_insn_page[0] = 0x48; // mov $0xc3c3c3c3c3c3c3c3, %rcx + alt_insn_page[1] = 0xb9; + + // Load the code TLB with insn_page, but point the page tables at + // alt_insn_page (and keep the data TLB clear, for AMD decode assist). + // This will make the CPU trap on the insn_page instruction but the + // hypervisor will see alt_insn_page. + install_page(cr3, virt_to_phys(insn_page), insn_ram); + // Load code TLB + invlpg(insn_ram); + asm volatile("call *%0" : : "r"(insn_ram + 3)); + // Trap, let hypervisor emulate at alt_insn_page + install_page(cr3, virt_to_phys(alt_insn_page), insn_ram); + asm volatile("call *%1" : "=c"(val) : "r"(insn_ram), "a"(mem), "c"(0)); + report("64-bit mov imm", val == 0xc3c3c3c3c3c3c3c3); +} + static void test_crosspage_mmio(volatile uint8_t *mem) { volatile uint16_t w, *pw; @@ -759,6 +791,7 @@ int main() test_shld_shrd(mem); test_mmx_movq_mf(mem, insn_page, alt_insn_page, insn_ram); + test_movabs(mem, insn_page, alt_insn_page, insn_ram); test_crosspage_mmio(mem);
MOV immediate instruction (opcodes 0xB8-0xBF) may take 64-bit operand. Some hypervisor implementations assumed the operand is 32-bit. This should never happen because the instruction has no memory operand, but (like the existing test_mmx_movq_mf) the testcase tricks the emulator into executing one by mismatching the page tables and the corresponding TLB entry. Cc: Nadav Amit <nadav.amit@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- x86/emulator.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)