Re: x86 emulation bug

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.04.2025 00:52, Fabian Specht wrote:
> Dear all,
> 
> we discovered a bug in hvm64 x86 Xen. Using the attached xtf test we are
> able to trigger an assert in arch/x86/hvm/emulate.c:
> 
> 
> if ( (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (reps > 1) )
> {
>       /*
>        * x86_emulate() clips the repetition count to ensure we don't wrap
>        * the effective-address index register. Hence this assertion holds.
>        */
>       ASSERT(offset >= ((reps - 1) * bytes_per_rep));
>       okay = hvm_virtual_to_linear_addr(
>               seg, reg, offset - (reps - 1) * bytes_per_rep,
>               reps * bytes_per_rep, access_type,
>               hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt), linear);
>       *linear += (reps - 1) * bytes_per_rep;
>       if ( hvmemul_ctxt->ctxt.addr_size != 64 )
>               *linear = (uint32_t)*linear;
> }
> 
> 
> If debug mode is not enabled, this will later on result in an integer
> underflow, however we were not able to find any severe problems.
> 
> The test below will not work with vanilla xtf.

Thanks for the report. However, already on Andreas' report on the day before
yesterday I was heavily inclined to ask about details you certainly have
figured out already. I find it odd that you leave to us all re-figuring out.
In this case, for example: Is the specific value you set %eax to relevant?
(Probably not.) Is the specific value you set %rcx to relevant? (Probably
not, but likely some part of it is.) Is the specific value you set %rdi to
relevant? (Probably not, but likely some part of it is. Probably you
setting it to 0 is also why the adjustment below is needed, and using a
different value might permit avoiding that adjustment.) Is it relevant
that you set EFLAGS.DF? (Quite likely.)

Any other investigation results you may have collected would likely also
make our job easier.

Jan

> To get it working,
> we have to edit 'pae_l1_identmap' in arch/x86/hvm/pagetables.S in xtf
> from
> 
> 
> PAGETABLE_START(pae_l1_identmap)
>         .long 0, 0
>         .rept PAE_L1_PT_ENTRIES - 1
>         .long (PAE_IDX(pae_l1_identmap) << PAE_L1_PT_SHIFT) + _PAGE_LEAF
>         .long 0
>         .endr
> PAGETABLE_END(pae_l1_identmap)
> 
> 
> to the following value:
> 
> 
> PAGETABLE_START(pae_l1_identmap)
>         .quad (0x5564000000) + _PAGE_LEAF
>         .rept PAE_L1_PT_ENTRIES - 1
>         .long (PAE_IDX(pae_l1_identmap) << PAE_L1_PT_SHIFT) + _PAGE_LEAF
>         .long 0
>         .endr
> PAGETABLE_END(pae_l1_identmap)
> 
> 
> 
> We were able to reproduce this bug in several versions including the
> most current one at the time of writing this.
> 
> 
> Best,
> Fabian