|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/x86/svm: Apply the virtual-8086 IOPL check to INT n only
On 22.10.2026 22:29, Stefano Stabellini wrote:
> On Thu, 22 Oct 2026, Andrew Cooper wrote:
>> On 22/10/2026 2:55 pm, Ross Lagerwall wrote:
>>> On 10/20/26 11:41 PM, Stefano Stabellini wrote:
>>>> In virtual-8086 mode with IOPL < 3, INT n raises #GP(0). INT1, INT3
>>>> and INTO are not affected. They are delivered normally.
>>>>
>>>> svm_emul_swint_injection() applies the check to all of them, so an ICEBP
>>>> raises #GP instead of #DB. It also reports the IDT entry in the #GP
>>>> error code instead of 0.
>>>>
>>>> Limit the check to INT n and use error code 0.
>>>>
>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
>>>> ---
>>>> xen/arch/x86/hvm/svm/svm.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>>>> index 71e48351f2..df74531d1a 100644
>>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>>> @@ -1153,9 +1153,12 @@ static void svm_emul_swint_injection(struct
>>>> x86_event *event)
>>>> * 32bit OS. Someone with many TUITs can see about reading the
>>>> TSS
>>>> * Software Interrupt Redirection bitmap.
>>>> */
>>>> - if ( (regs->eflags & X86_EFLAGS_VM) &&
>>>> + if ( type == X86_ET_SW_INT && (regs->eflags & X86_EFLAGS_VM) &&
>>>> MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
>>>> + {
>>>> + ec = 0;
>>>> goto raise_exception;
>>>> + }
>>>> /*
>>>> * Read all 8/16 bytes so the idtr limit check is applied
>>>> properly to
>>>
>>> I think this is correct. The check could perhaps move earlier to avoid
>>> needing
>>> to reset ec to 0.
>>>
>>> I do wonder though if there are other cases in the function where ec
>>> needs to
>>> be reset to 0? The pseudocode in the APM seems to have at least one
>>> other case.
>>
>> I confirmed, and indeed it is #GP[0] not #GP[sel]. Also that
>> ICEBP/INT3/INTO are not subject IOPL like this.
>>
>> I agree with Ross that this wants moving earlier; between the CR0.PE
>> check and deciding idte_size. The comment also wants adjusting, so I
>> think it wants to look like this:
>>
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index 71e48351f22c..c83b28d29727 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1140,6 +1140,17 @@ static void svm_emul_swint_injection(struct
>> x86_event *event)
>> if ( !(vmcb_get_cr0(vmcb) & X86_CR0_PE) )
>> goto raise_exception; /* TODO: support real-mode injection? */
>>
>> + /*
>> + * In vm86 mode, INT $N is subject to IOPL.
>> + *
>> + * TODO: It is also subject to CR4.VME=1, which would require
>> reading the
>> + * TSS Software Interrupt Redirection bitmap.
>> + */
>> + if ( type == X86_ET_SW_INT &&
>> + regs->eflags & X86_EFLAGS_VM &&
>> + MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
>> + goto raise_exception;
>> +
>> idte_size = lm ? 16 : 8;
>> idte_offset = trap * idte_size;
>>
>> @@ -1147,16 +1158,6 @@ static void svm_emul_swint_injection(struct
>> x86_event *event)
>> ec = (trap << 3) | X86_XEC_IDT |
>> (type == X86_ET_PRIV_SW_EXC ? X86_XEC_EXT : 0);
>>
>> - /*
>> - * TODO: This does not cover the v8086 mode with CR4.VME case
>> - * correctly, but falls on the safe side from the point of view of a
>> - * 32bit OS. Someone with many TUITs can see about reading the TSS
>> - * Software Interrupt Redirection bitmap.
>> - */
>> - if ( (regs->eflags & X86_EFLAGS_VM) &&
>> - MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
>> - goto raise_exception;
>> -
>> /*
>> * Read all 8/16 bytes so the idtr limit check is applied properly to
>> * this entry, even though we don't look at all the words read.
>>
>>
>> I'm happy to adjust this on commit if you're happy?
>
> Yes please, thank you!
Can we also have
Fixes: 897d59a426f1 ("x86/svm: Introduce svm_emul_swint_injection()")
then, please?
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |