SVM: svm_get_insn_len() improvements

Commit Message

Jan Beulich April 18, 2023, 10:33 a.m. UTC

Don't let x86_decode_insn() failing go silently.

Check hardware provided value (if sensible) against decoder provided
one. Also use it as return value on the error path - there's no real
reason to inject #GP if we have a presumably good value in hands.

Check that, when no ModR/M byte is expected, the decoder also didn't
think there is one. This makes things symmetric with the opposite case,
where there being a valid ModR/M byte is implictly checked by the first
of the involved comparisons.

While adding the initializers, also switch emul_len to "unsigned int",
matching both the function's return type and that of x86_insn_length().

Signed-off-by: Jan Beulich <[e-mail]>

Patch

--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -56,8 +56,8 @@  unsigned int svm_get_insn_len(struct vcp
 {
     struct hvm_emulate_ctxt ctxt;
     struct x86_emulate_state *state;
-    unsigned long nrip_len, emul_len;
-    unsigned int instr_opcode, instr_modrm;
+    unsigned long nrip_len;
+    unsigned int emul_len = 0, instr_opcode = 0, instr_modrm = 0;
     unsigned int modrm_rm, modrm_reg;
     int modrm_mod;
 
@@ -75,19 +75,22 @@  unsigned int svm_get_insn_len(struct vcp
     hvm_emulate_init_per_insn(&ctxt, NULL, 0);
     state = x86_decode_insn(&ctxt.ctxt, hvmemul_insn_fetch);
     if ( IS_ERR_OR_NULL(state) )
-        return 0;
+        goto bad;
 
     emul_len = x86_insn_length(state, &ctxt.ctxt);
     modrm_mod = x86_insn_modrm(state, &modrm_rm, &modrm_reg);
     x86_emulate_free_state(state);
 
+    if ( nrip_len > 0 && nrip_len <= MAX_INST_LEN && emul_len != nrip_len )
+        goto bad;
+
     /* Extract components from instr_enc. */
     instr_modrm  = instr_enc & 0xff;
     instr_opcode = instr_enc >> 8;
 
     if ( instr_opcode == ctxt.ctxt.opcode )
     {
-        if ( !instr_modrm )
+        if ( !instr_modrm && modrm_mod < 0 )
             return emul_len;
 
         if ( modrm_mod       == MASK_EXTR(instr_modrm, 0300) &&
@@ -96,12 +99,16 @@  unsigned int svm_get_insn_len(struct vcp
             return emul_len;
     }
 
+ bad:
     printk(XENLOG_G_WARNING
-           "Insn mismatch: Expected opcode %#x, modrm %#x, got nrip_len %lu, emul_len %lu\n",
+           "Insn mismatch: Expected opcode %#x, modrm %#x, got nrip_len %lu, emul_len %u\n",
            instr_opcode, instr_modrm, nrip_len, emul_len);
     hvm_dump_emulation_state(XENLOG_G_WARNING, "SVM Insn len",
                              &ctxt, X86EMUL_UNHANDLEABLE);
 
+    if ( nrip_len > 0 && nrip_len <= MAX_INST_LEN )
+        return nrip_len;
+
     hvm_inject_hw_exception(X86_EXC_GP, 0);
     return 0;
 }