Re: [Xen-devel] [PATCH] x86/pv: Fix guest crashes following f75b1a5247b "x86/pv: Drop int80_bounce from struct pv_vcpu"

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/03/18 13:58, Jan Beulich wrote:
>>>> On 14.03.18 at 12:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>> The original init_int80_direct_trap() was in fact buggy; `int $0x80` is not 
>> an
>> exception.  This went unnoticed for years because int80_bounce and 
>> trap_bounce
>> were separate structures, but were combined by this change.
>>
>> Exception handling is different to interrupt handling for PV guests.  By
>> reusing trap_bounce, the following corner case can occur:
>>
>>  * Handle a guest `int $0x80` instruction.  Latches TBF_EXCEPTION into
>>    trap_bounce.
>>  * Handle an exception, which emulates to success (such as ptwr support),
>>    which leaves trap_bounce unmodified.
>>  * The exception exit path sees TBF_EXCEPTION set and re-injects the `int
>>    $0x80` a second time.
> Oh, and then it was the clearing of trap_bounce after consuming it
> in your conversion to C which masked the problem?

Yes.

>
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -373,10 +373,10 @@ UNLIKELY_END(msi_check)
>>          mov   %cx, TRAPBOUNCE_cs(%rdx)
>>          mov   %rdi, TRAPBOUNCE_eip(%rdx)
>>  
>> -        /* TB_flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); 
>> */
>> +        /* TB_flags = (TI_GET_IF(ti) ? TBF_INTERRUPT : 0); */
>>          testb $4, 0x80 * TRAPINFO_sizeof + TRAPINFO_flags(%rsi)
>>          setnz %cl
>> -        lea   TBF_EXCEPTION(, %rcx, TBF_INTERRUPT), %ecx
>> +        lea   (, %rcx, TBF_INTERRUPT), %ecx
> With the immediate gone I think
>
>     shl   $3, %ecx
>
> would be more readable and perhaps no worse code wise (the
> use of LEA was introduced in cases like this only to combine the
> shift with the ORing in of other flags). I won't insist on that
> change though (the more that there's no symbolic constant
> available for that literal 3 right now), so with or without it
>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

I'll do a followup patch.  This particular pattern exists elsewhere, so
might as well fix them all.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel