Re: [PATCH 01/21] KVM: x86: decode_modrm does not regard modrm correctly

[Paolo Bonzini <pbonzini@redhat.com>]

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