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.
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
