Re: [PATCH] x86emul: convert op_bytes/opc checks in SIMD emulation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/08/2024 2:30 pm, Jan Beulich wrote:
> Delivering #UD for an internal shortcoming of the emulator isn't quite
> right. Similarly BUG() is bigger a hammer than needed.
>
> Switch to using EXPECT() instead.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

To confirm, this is ASSERT_UNREACHABLE() (which fuzzing will now notice
as an error), and unhandleable in release builds (which ultimately ends
up in #UD)?

I think it would be helpful to at least note the fuzzing aspect in the
commit message.

>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -8114,13 +8114,13 @@ x86_emulate(
>      }
>      else if ( state->simd_size != simd_none )
>      {
> -        generate_exception_if(!op_bytes, X86_EXC_UD);
>          generate_exception_if((vex.opcx && (d & TwoOp) &&
>                                 (vex.reg != 0xf || (evex_encoded() && 
> !evex.RX))),
>                                X86_EXC_UD);
>  
> -        if ( !opc )
> -            BUG();
> +        EXPECT(op_bytes);
> +        EXPECT(opc);

This is the only BUG() in x86_emulate.c, and it's right to get rid of it
IMO.

Therefore, we should have a hunk removing it from
tools/tests/x86_emulator/x86-emulate.h too, which will prevent
reintroduction.

Maybe even undef BUG somewhere in x86_emulate/private.h?

~Andrew