Re: [PATCH] x86emul/fuzz: add a state sanitization function

[George Dunlap <George.Dunlap@citrix.com>]

> On Mar 29, 2019, at 2:54 PM, Jan Beulich <JBeulich@suse.com> wrote:
> 
> This is to accompany sanitize_input(). Just like for initial state we
> want to have state between two emulated insns sane, at least as far as
> assumptions in the main emulator go. Do minimal checking after segment
> register, CR, and MSR writes, and roll back to the old value in case of
> failure (raising #GP(0) at the same time).
> 
> In the particular case observed, a CR0 write clearing CR0.PE was
> followed by a VEX-encoded insn, which the decoder accepts based on
> guest address size, restricting things just outside of the 64-bit case
> (real and virtual modes don't allow VEX-encoded insns). Subsequently
> _get_fpu() would then assert that CR0.PE must be set (and EFLAGS.VM
> clear) when trying to invoke YMM, ZMM, or OPMASK state.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -76,6 +76,8 @@ static inline bool input_read(struct fuz
>     return true;
> }
> 
> +static bool sanitize_state(struct x86_emulate_ctxt *ctxt);
> +
> static const char* const x86emul_return_string[] = {
>     [X86EMUL_OKAY] = "X86EMUL_OKAY",
>     [X86EMUL_UNHANDLEABLE] = "X86EMUL_UNHANDLEABLE",
> @@ -424,8 +426,19 @@ static int fuzz_write_segment(
>     rc = maybe_fail(ctxt, "write_segment", true);
> 
>     if ( rc == X86EMUL_OKAY )
> +    {
>         c->segments[seg] = *reg;
> 
> +        if ( !sanitize_state(ctxt) )
> +        {
> +            struct segment_register old = c->segments[seg];

I think you have this in the wrong place.

Everything else looks good.

 -George


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