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

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

 




> -----Original Message-----
> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On
> Behalf Of Nadav Amit
> Sent: Thursday, December 25, 2014 5:55 PM
> To: Chen, Tiejun
> Cc: Paolo Bonzini; kvm list
> Subject: Re: [PATCH 2/8] KVM: x86: pop sreg accesses only 2 bytes
> 
> Tiejun <tiejun.chen@xxxxxxxxx> wrote:
> 
> > 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@xxxxxxxxxxxxxxxxx>
> >> ---
> >>  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?
> I don't think so. It seems the behaviour of push and pop is a bit different.
> For push: "If the source operand is a segment register (16 bits) and the
> operand size is 64-bits, a zero-extended value is pushed on the stack; if
> the operand size is 32-bits ... all recent Core and Atom processors perform
> a 16-bit move, leaving the upper portion of the stack location unmodified."
> 
> Therefore, for push in the case of op_bytes==8 we push zero-extended value.
> 
> For pop the behaviour is not well-documented, but experimentally it appears
> only the first two bytes are accessed. I cannot see why it would be
> different when opsize is 8, since it is not like the push case, where the
> segment register value was zero extended.

Let's take 64-bits operand size as an example. When pushing a segment register, it
uses zero-extended value, so 8 bytes will be pushed on the stack. When popping it,
the current code return the top 8 bytes in the stack, and it only uses the lowest 2
bytes for load_segment_descriptor(). what is the issue here?

Thanks,
Feng

> 
> If you feel strongly about it, I'll create a unit test.
> 
> Nadav
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux