From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 01/21] KVM: x86: decode_modrm does not regard modrm correctly
Date: Wed, 05 Nov 2014 12:14:28 +0100
Message-ID: <545A0694.7060108@redhat.com>
References: <1414922101-17626-1-git-send-email-namit@cs.technion.ac.il> <1414922101-17626-2-git-send-email-namit@cs.technion.ac.il>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, nadav.amit@gmail.com
To: Nadav Amit <namit@cs.technion.ac.il>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:37555 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751403AbaKELOl (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 5 Nov 2014 06:14:41 -0500
In-Reply-To: <1414922101-17626-2-git-send-email-namit@cs.technion.ac.il>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 02/11/2014 10:54, Nadav Amit wrote:
> In one occassion, decode_modrm uses the rm field after it is extended with
> REX.B to determine the addressing mode. Doing so causes it not to read the
> offset for rip-relative addressing.
> 
> This patch uses the value after masking instead.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  arch/x86/kvm/emulate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 4b3fa70..efe7239 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1209,7 +1209,7 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>  		}
>  		switch (ctxt->modrm_mod) {
>  		case 0:
> -			if (ctxt->modrm_rm == 5)
> +			if ((ctxt->modrm_rm & 7) == 5)
>  				modrm_ea += insn_fetch(s32, ctxt);
>  			break;
>  		case 1:
> 

We can do instead:

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 05ea85ff39c6..a4703eb9c1ed 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1223,6 +1223,7 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
 			if (index_reg != 4)
 				modrm_ea += reg_read(ctxt, index_reg) << scale;
 		} else if ((ctxt->modrm_rm & 7) == 5 && ctxt->modrm_mod == 0) {
+			modrm_ea += insn_fetch(s32, ctxt);
 			if (ctxt->mode == X86EMUL_MODE_PROT64)
 				ctxt->rip_relative = 1;
 		} else {
@@ -1231,10 +1232,6 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
 			adjust_modrm_seg(ctxt, base_reg);
 		}
 		switch (ctxt->modrm_mod) {
-		case 0:
-			if ((ctxt->modrm_rm & 7) == 5)
-				modrm_ea += insn_fetch(s32, ctxt);
-			break;
 		case 1:
 			modrm_ea += insn_fetch(s8, ctxt);
 			break;

which matches what we already do to handle the case where REX.B is
ignored for MOD=00, RM=0100:

                        if ((base_reg & 7) == 5 && ctxt->modrm_mod == 0)

I'll write a testcase, too.

Paolo