i#6238: validate instructions: push,pop,cmp,pusha,popa,push_imm,ret,r…

…et_far,loopne,loope,loop,movzx,cmp,rcr (#6331) Add instructions to categories: OP_push,OP_pop,OP_cmp,OP_pusha,OP_popa,OP_push_imm,OP_ret,OP_ret_far,OP_loopne,OP_loope,OP_loop,OP_movzx,OP_cmp,OP_rcr Fix: Moved category decoding after operands decoding to check STORE/LOAD memory access Issue: #6238
DynamoRIO · Sep 27, 2023 · c9b0634 · c9b0634
1 parent 5fbde59
commit c9b0634
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 63 deletions.
diff --git a/core/ir/x86/decode.c b/core/ir/x86/decode.c
@@ -2447,10 +2447,20 @@ decode_category(instr_t *instr)
         if (op_instr[instr->opcode] != NULL) {
             uint category = op_instr[instr->opcode]->category;
             if (instr_operands_valid(instr)) {
-                if (instr_reads_memory(instr))
+                if (instr_reads_memory(instr)) {
                     category |= DR_INSTR_CATEGORY_LOAD;
-                if (instr_writes_memory(instr))
+                    if (TEST(DR_INSTR_CATEGORY_MOVE, category)) {
+                        category &= ~DR_INSTR_CATEGORY_MOVE;
+                        category &= ~DR_INSTR_CATEGORY_FP;
+                    }
+                }
+                if (instr_writes_memory(instr)) {
                     category |= DR_INSTR_CATEGORY_STORE;
+                    if (TEST(DR_INSTR_CATEGORY_MOVE, category)) {
+                        category &= ~DR_INSTR_CATEGORY_MOVE;
+                        category &= ~DR_INSTR_CATEGORY_FP;
+                    }
+                }
             }
             instr_set_category(instr, category);
         } else {
@@ -2591,7 +2601,6 @@ decode_common(dcontext_t *dcontext, byte *pc, byte *orig_pc, instr_t *instr)
                                         decode operands too */
                                _IF_DEBUG(!TEST(INSTR_IGNORE_INVALID, instr->flags)));
     instr_set_opcode(instr, info->type);
-    decode_category(instr);
     IF_X64(instr_set_x86_mode(instr, di.x86_mode));
     /* failure up to this point handled fine -- we set opcode to OP_INVALID */
     if (next_pc == NULL) {
@@ -2736,6 +2745,8 @@ decode_common(dcontext_t *dcontext, byte *pc, byte *orig_pc, instr_t *instr)
         instr_set_rip_rel_pos(instr, (int)(di.disp_abs - di.start_pc));
     }
 
+    decode_category(instr);
+
     return next_pc;
 
 decode_invalid: