[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes |
Date: |
Wed, 18 Oct 2017 18:12:07 +0200 |
Besides being more correct, arbitrarily long instruction allow the
generation of a translation block that spans three pages. This
confuses the generator and even allows ring 3 code to poison the
translation block cache and inject code into other processes that are
in guest ring 3.
This is an improved (and more invasive) fix for commit 30663fd ("tcg/i386:
Check the size of instruction being translated", 2017-03-24). In addition
to being more precise (and generating the right exception, which is #GP
rather than #UD), it distinguishes better between page faults and too long
instructions, as shown by this test case:
#include <sys/mman.h>
#include <string.h>
#include <stdio.h>
int main()
{
char *x = mmap(NULL, 8192, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_PRIVATE|MAP_ANON, -1, 0);
memset(x, 0x66, 4096);
x[4096] = 0x90;
x[4097] = 0xc3;
char *i = x + 4096 - 15;
mprotect(x + 4096, 4096, PROT_READ|PROT_WRITE);
((void(*)(void)) i) ();
}
... which produces a #GP without the mprotect, and a #PF with it.
Signed-off-by: Paolo Bonzini <address@hidden>
---
target/i386/translate.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 4a938c2..5f24a2d 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -136,6 +136,7 @@ typedef struct DisasContext {
int cpuid_ext3_features;
int cpuid_7_0_ebx_features;
int cpuid_xsave_features;
+ sigjmp_buf jmpbuf;
} DisasContext;
static void gen_eob(DisasContext *s);
@@ -1863,11 +1864,27 @@ static void gen_shifti(DisasContext *s1, int op,
TCGMemOp ot, int d, int c)
}
}
+#define X86_MAX_INSN_LENGTH 15
+
static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
{
uint64_t pc = s->pc;
s->pc += num_bytes;
+ if (unlikely(s->pc - s->pc_start > X86_MAX_INSN_LENGTH)) {
+ /* If the instruction's 16th byte is on a different page than the 1st,
a
+ * page fault on the second page wins over the general protection fault
+ * caused by the instruction being too long.
+ * This can happen even if the operand is only one byte long!
+ */
+ if (((s->pc - 1) ^ (pc - 1)) & TARGET_PAGE_MASK) {
+ volatile uint8_t unused =
+ cpu_ldub_code(env, (s->pc - 1) & TARGET_PAGE_MASK);
+ (void) unused;
+ }
+ siglongjmp(s->jmpbuf, 1);
+ }
+
return pc;
}
@@ -4463,14 +4480,12 @@ static target_ulong disas_insn(DisasContext *s,
CPUState *cpu)
s->rip_offset = 0; /* for relative ip address */
s->vex_l = 0;
s->vex_v = 0;
- next_byte:
- /* x86 has an upper limit of 15 bytes for an instruction. Since we
- * do not want to decode and generate IR for an illegal
- * instruction, the following check limits the instruction size to
- * 25 bytes: 14 prefix + 1 opc + 6 (modrm+sib+ofs) + 4 imm */
- if (s->pc - pc_start > 14) {
- goto illegal_op;
+ if (sigsetjmp(s->jmpbuf, 0) != 0) {
+ gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+ return s->pc;
}
+
+ next_byte:
b = x86_ldub_code(env, s);
/* Collect prefixes. */
switch (b) {
--
1.8.3.1
- [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one(), (continued)
- [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one(), Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 05/29] exec: add page_mask for flatview_do_translate, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 03/29] ide: support reporting of rotation rate, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 06/29] exec: simplify address_space_get_iotlb_entry, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 08/29] pc: make sure that plugged CPUs are of the same type, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 09/29] disas: Always initialize read_memory_inner_func properly, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 10/29] build: remove CONFIG_LIBDECNUMBER, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 13/29] tco: add trace events, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 11/29] nios2: define tcg_env, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 17/29] kvm: fix alignment of ram address, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes,
Paolo Bonzini <=
- [Qemu-devel] [PULL 12/29] docs/devel/loads-stores.rst: Document our various load and store APIs, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 14/29] target/i386: introduce x86_ld*_code, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 19/29] kvm: fix error message when failing to unregister slot, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 16/29] memory: call log_start after region_add, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 18/29] kvm: tolerate non-existing slot for log_start/log_stop/log_sync, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 22/29] memory: reuse section_from_flat_range(), Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 21/29] kvm: simplify kvm_align_section(), Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 20/29] kvm: region_add and region_del is not called on updates, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 24/29] watch_mem_write: implement 8-byte accesses, Paolo Bonzini, 2017/10/18
- [Qemu-devel] [PULL 25/29] qemu-pr-helper: use new libmultipath API, Paolo Bonzini, 2017/10/18