[Qemu-devel] [RFC][PATCH] x86: CS limit checks

[Jan Kiszka <jan.kiszka@siemens.com>]

Here is a proposal for adding code segment limit checks to x86. This
patch should not need the -seg-checks switch as its tests are mostly
performed during translation time. Moreover, I tried to confine the
small additional overhead in the TB lookup procedure to x86 and Sparc.

Note that this patch depends on my debugging series, namely [1], as that
one reduces the x86-specific code passages for TB generation. Also note
that this patch is early and only lightly tested so far, not yet
intended for inclusion, but definitely for commenting on!

Jan

[1] http://permalink.gmane.org/gmane.comp.emulators.qemu/26993

---
 cpu-exec.c              |   23 ++++++++++++++---------
 exec-all.h              |   29 +++++++++++++++++++++++++++--
 exec.c                  |    8 +++++---
 target-i386/translate.c |   15 ++++++++++++---
 4 files changed, 58 insertions(+), 17 deletions(-)

Index: b/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -78,8 +78,8 @@ int cpu_restore_state_copy(struct Transl
 void cpu_resume_from_signal(CPUState *env1, void *puc);
 void cpu_io_recompile(CPUState *env, void *retaddr);
 TranslationBlock *tb_gen_code(CPUState *env, 
-                              target_ulong pc, target_ulong cs_base, int flags,
-                              int cflags);
+                              target_ulong pc, target_ulong cs_base,
+                              target_ulong cs_limit, int flags, int cflags);
 void cpu_exec_init(CPUState *env);
 int page_unprotect(target_ulong address, unsigned long pc, void *puc);
 void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
@@ -125,6 +125,7 @@ static inline int tlb_set_page(CPUState
 struct TranslationBlock {
     target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
     target_ulong cs_base; /* CS base for this block */
+    target_ulong cs_limit; /* CS limit for this block */
     uint64_t flags; /* flags defining in which context the code was generated */
     uint16_t size;      /* size of target code for this block (1 <=
                            size <= TARGET_PAGE_SIZE) */
@@ -157,6 +158,30 @@ struct TranslationBlock {
     uint32_t icount;
 };
 
+#if defined(TARGET_I386) || defined(TARGET_SPARC)
+static inline int tb_cs_base_equal(TranslationBlock *tb, target_ulong cs_base)
+{
+    return (tb->cs_base == cs_base);
+}
+#else
+static inline int tb_cs_base_equal(TranslationBlock *tb, target_ulong cs_base)
+{
+    return 1;
+}
+#endif
+
+#if defined(TARGET_I386)
+static inline int tb_cs_limit_equal(TranslationBlock *tb, target_ulong cs_limit)
+{
+    return (tb->cs_limit == cs_limit);
+}
+#else
+static inline int tb_cs_limit_equal(TranslationBlock *tb, target_ulong cs_limit)
+{
+    return 1;
+}
+#endif
+
 static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc)
 {
     target_ulong tmp;
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -810,7 +810,7 @@ static void build_page_bitmap(PageDesc *
 
 TranslationBlock *tb_gen_code(CPUState *env,
                               target_ulong pc, target_ulong cs_base,
-                              int flags, int cflags)
+                              target_ulong cs_limit, int flags, int cflags)
 {
     TranslationBlock *tb;
     uint8_t *tc_ptr;
@@ -830,6 +830,7 @@ TranslationBlock *tb_gen_code(CPUState *
     tc_ptr = code_gen_ptr;
     tb->tc_ptr = tc_ptr;
     tb->cs_base = cs_base;
+    tb->cs_limit = cs_limit;
     tb->flags = flags;
     tb->cflags = cflags;
     cpu_gen_code(env, tb, &code_gen_size);
@@ -3133,7 +3134,7 @@ void cpu_io_recompile(CPUState *env, voi
 {
     TranslationBlock *tb;
     uint32_t n, cflags;
-    target_ulong pc, cs_base;
+    target_ulong pc, cs_base, cs_limit;
     uint64_t flags;
 
     tb = tb_find_pc((unsigned long)retaddr);
@@ -3173,11 +3174,12 @@ void cpu_io_recompile(CPUState *env, voi
     cflags = n | CF_LAST_IO;
     pc = tb->pc;
     cs_base = tb->cs_base;
+    cs_limit = tb->cs_limit;
     flags = tb->flags;
     tb_phys_invalidate(tb, -1);
     /* FIXME: In theory this could raise an exception.  In practice
        we have already translated the block once so it's probably ok.  */
-    tb_gen_code(env, pc, cs_base, flags, cflags);
+    tb_gen_code(env, pc, cs_base, cs_limit, flags, cflags);
     /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
        the first in the TB) then we end up generating a whole new TB and
        repeating the fault, which is horribly inefficient.
Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -94,8 +94,8 @@ static void cpu_exec_nocache(int max_cyc
     if (max_cycles > CF_COUNT_MASK)
         max_cycles = CF_COUNT_MASK;
 
-    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
-                     max_cycles);
+    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->cs_limit,
+                     orig_tb->flags, max_cycles);
     env->current_tb = tb;
     /* execute the generated code */
     next_tb = tcg_qemu_tb_exec(tb->tc_ptr);
@@ -109,9 +109,8 @@ static void cpu_exec_nocache(int max_cyc
     tb_free(tb);
 }
 
-static TranslationBlock *tb_find_slow(target_ulong pc,
-                                      target_ulong cs_base,
-                                      uint64_t flags)
+static TranslationBlock *tb_find_slow(target_ulong pc, target_ulong cs_base,
+                                      target_ulong cs_limit, uint64_t flags)
 {
     TranslationBlock *tb, **ptb1;
     unsigned int h;
@@ -133,7 +132,8 @@ static TranslationBlock *tb_find_slow(ta
             goto not_found;
         if (tb->pc == pc &&
             tb->page_addr[0] == phys_page1 &&
-            tb->cs_base == cs_base &&
+            tb_cs_base_equal(tb, cs_base) &&
+            tb_cs_limit_equal(tb, cs_limit) &&
             tb->flags == flags) {
             /* check next page if needed */
             if (tb->page_addr[1] != -1) {
@@ -150,7 +150,7 @@ static TranslationBlock *tb_find_slow(ta
     }
  not_found:
    /* if no translated code available, then translate it now */
-    tb = tb_gen_code(env, pc, cs_base, flags, env->next_cflags);
+    tb = tb_gen_code(env, pc, cs_base, cs_limit, flags, env->next_cflags);
     env->next_cflags = 0;
 
  found:
@@ -163,6 +163,7 @@ static inline TranslationBlock *tb_find_
 {
     TranslationBlock *tb;
     target_ulong cs_base, pc;
+    target_ulong cs_limit = (target_ulong)-1;
     uint64_t flags;
 
     /* we record a subset of the CPU state. It will
@@ -172,6 +173,8 @@ static inline TranslationBlock *tb_find_
     flags = env->hflags;
     flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
     cs_base = env->segs[R_CS].base;
+    if ((env->hflags & (HF_PE_MASK | HF_CS64_MASK)) == HF_PE_MASK)
+        cs_limit = env->segs[R_CS].limit;
     pc = cs_base + env->eip;
 #elif defined(TARGET_ARM)
     flags = env->thumb | (env->vfp.vec_len << 1)
@@ -225,9 +228,11 @@ static inline TranslationBlock *tb_find_
 #error unsupported CPU
 #endif
     tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
-    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
+    if (unlikely(!tb || tb->pc != pc ||
+                 !tb_cs_base_equal(tb, cs_base) ||
+                 !tb_cs_limit_equal(tb, cs_limit) ||
                  tb->flags != flags)) {
-        tb = tb_find_slow(pc, cs_base, flags);
+        tb = tb_find_slow(pc, cs_base, cs_limit, flags);
     }
     return tb;
 }
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -81,6 +81,7 @@ typedef struct DisasContext {
                    static state change (stop translation) */
     /* current block context */
     target_ulong cs_base; /* base of CS segment */
+    target_ulong cs_limit; /* limit of the CS segment */
     int pe;     /* protected mode */
     int code32; /* 32 bit code segment */
 #ifdef TARGET_X86_64
@@ -3687,6 +3688,8 @@ static target_ulong disas_insn(DisasCont
     int modrm, reg, rm, mod, reg_addr, op, opreg, offset_addr, val, seg_reg;
     target_ulong next_eip, tval;
     int rex_w, rex_r;
+    uint16_t *gen_opc_start = gen_opc_ptr;
+    TCGArg *gen_opparam_start = gen_opparam_ptr;
 
     if (unlikely(loglevel & CPU_LOG_TB_OP))
         tcg_gen_debug_insn_start(pc_start);
@@ -7270,6 +7273,13 @@ static target_ulong disas_insn(DisasCont
     /* lock generation */
     if (s->prefix & PREFIX_LOCK)
         tcg_gen_helper_0_0(helper_unlock);
+    if (s->pc < s->cs_base || s->pc - s->cs_base > s->cs_limit) {
+        /* At least some of the opcode fetches violate the CS limit.
+           Overwrite the generated code with a GPF raising one. */
+        gen_opc_ptr = gen_opc_start;
+        gen_opparam_ptr = gen_opparam_start;
+        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+    }
     return s->pc;
  illegal_op:
     if (s->prefix & PREFIX_LOCK)
@@ -7316,13 +7326,11 @@ static inline int gen_intermediate_code_
     int j, lj, cflags;
     uint64_t flags;
     target_ulong pc_start;
-    target_ulong cs_base;
     int num_insns;
     int max_insns;
 
     /* generate intermediate code */
     pc_start = tb->pc;
-    cs_base = tb->cs_base;
     flags = tb->flags;
     cflags = tb->cflags;
 
@@ -7337,7 +7345,8 @@ static inline int gen_intermediate_code_
     dc->tf = (flags >> TF_SHIFT) & 1;
     dc->singlestep_enabled = env->singlestep_enabled;
     dc->cc_op = CC_OP_DYNAMIC;
-    dc->cs_base = cs_base;
+    dc->cs_base = tb->cs_base;
+    dc->cs_limit = tb->cs_limit;
     dc->tb = tb;
     dc->popl_esp_hack = 0;
     /* select memory access functions */