[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] SVM: svm_get_insn_len() improvements


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 18 Apr 2023 12:33:37 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ho7DQTfcrRGgvdcXsi/AG6ZA4fuFOVAjl3n8bd4GpfQ=; b=LR+wdnEQpUnMsheH8hFbZ0v0AXE7cirouxPSjwI4yARBNrVuo6o5TbGn3BSaOaVRzkVECQPTb3hh1bYPpUrY9ADCHiw8vHeKMR7KLG3JQeVaTViDzfV4Y+O+Jv0UYAscIvogVgxt2zEZKk60TXT74iRLCWqJPKbuKpqKb2klYKB8Gt6c2BjoDp/FujRxpYIbfwRR6jYAmy1Imu4vjx0xhXjvIJv/kcmlKdc9q0rEaSpBYZ+lgpA7MM5/y34YDTTMZZglPQcXNuO4g+qKbOwhV8+nxdI2ULXsXDaKcLFenjgYCzVx/b+JJcXVh895clB47EtjvWNTwCZ3Ls222C4oIg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MyPKMuuaePvTIe3zjSuAEyroFYPdGjwxQ60Nr8j2m9jXKJ61cdQR+VcXQoEp+vEt9mcGHPrOOZqEsK3wNgnLEhCPvXIKecopAnwtvMyZwX/QQp0HLb+6n1fiGHrOsht3+s0X5d2Nn69T/9n6+tJ+dyY61gzHukyvsqhEZW9dk9dkJH+a2Os6jAzoDJzV193wEwn2YEQbU5x+0rzvCN35KbB3JnqoEgSs6XLkbtG3524VtpwnnaaISfzRevYZXGrwISo9yoGK3SOHGlidhNzdDKFrBfHf3OiXNPACTdZA0Q3crRO6k7wwIpuN4vo+4E8wvTTvOCK3zir8rpvoZnOQ/A==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Tue, 18 Apr 2023 10:34:26 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>

--- 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;
 }



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.