Re: [PATCH 2/8] KVM: x86: pop sreg accesses only 2 bytes

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2014/12/25 8:52, Nadav Amit wrote:
> Although pop sreg updates RSP according to the operand size, only 2 bytes are
> read.  The current behavior may result in incorrect #GP or #PF exceptions.
>
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>   arch/x86/kvm/emulate.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index e5a84be..702da5e 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1830,12 +1830,14 @@ static int em_pop_sreg(struct x86_emulate_ctxt *ctxt)
>   	unsigned long selector;
>   	int rc;
>

Looks we just should do similar thing to em_push_sreg(),

         unsigned long selector;
         int rc;

+       if (ctxt->op_bytes == 4) {
+               rsp_increment(ctxt, -2);
+               ctxt->op_bytes = 2;
+       }
         rc = emulate_pop(ctxt, &selector, ctxt->op_bytes);
         if (rc != X86EMUL_CONTINUE)
                 return rc;

Right?

Thanks
Tiejun

> -	rc = emulate_pop(ctxt, &selector, ctxt->op_bytes);
> +	rc = emulate_pop(ctxt, &selector, 2);
>   	if (rc != X86EMUL_CONTINUE)
>   		return rc;
>
>   	if (ctxt->modrm_reg == VCPU_SREG_SS)
>   		ctxt->interruptibility = KVM_X86_SHADOW_INT_MOV_SS;
> +	if (ctxt->op_bytes > 2)
> +		rsp_increment(ctxt, ctxt->op_bytes - 2);
>
>   	rc = load_segment_descriptor(ctxt, (u16)selector, seg);
>   	return rc;
>