On Mon, 14 Feb 2022 at 18:57, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > From: Cameron Esfahani <di...@apple.com> > > A guest could craft a specific stream of instructions that will have QEMU > write 0xF9 to inappropriate locations in memory. Add additional asserts > to check for this. Generate a #UD if there are more than 14 prefix bytes. > > Found by Julian Stecklina <julian.steckl...@cyberus-technology.de> > > Signed-off-by: Cameron Esfahani <di...@apple.com> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > --- > target/i386/hvf/x86_decode.c | 11 +++++++++-- > target/i386/hvf/x86hvf.c | 8 ++++++++ > target/i386/hvf/x86hvf.h | 1 + > 3 files changed, 18 insertions(+), 2 deletions(-)
I'm not a huge fan of the VM_PANIC_ON() macro, which appears to be essentially an obfuscated assert(), but it's the existing code style in this decoder I guess. > @@ -1847,7 +1849,8 @@ void calc_modrm_operand(CPUX86State *env, struct > x86_decode *decode, > > static void decode_prefix(CPUX86State *env, struct x86_decode *decode) > { > - while (1) { > + /* At most 14 prefix bytes. */ > + for (int i = 0; i < 14; i++) { > /* > * REX prefix must come after legacy prefixes. > * REX before legacy is ignored. > @@ -1892,6 +1895,8 @@ static void decode_prefix(CPUX86State *env, struct > x86_decode *decode) > return; > } > } > + /* Too many prefixes! Generate #UD. */ > + hvf_inject_ud(env); This doesn't look right. This is the decoder, so it shouldn't be actively affecting the state of the CPU. Also, we don't do anything here to cause us to stop trying to decode this instruction, so we'll carry on through the rest of decode and then the caller will call exec_instruction() on whatever results. I think if you want to say "this instruction should cause a #UD" you should emit an X86_DECODE_CMD_* for "raise #UD", stop decode of the insn at that point, and then handle that in exec_instruction(). You probably also need to take special care to avoid tripping the assert(ins_len == decode.len) at one of the callsites in hvf.c (or else just drop or modify that assert). thanks -- PMM