On Tue, Jun 25, 2024 at 11:37:19AM +0200, Peter Zijlstra wrote: > Also, wouldn't it be saner to write this something like: > > __always_inline int decode_bug(unsigned long addr, u32 *imm) > { > u8 v; > > if (addr < TASK_SIZE) > return BUG_NONE; > > v = *(u8 *)(addr++); > if (v == 0x67) > v = *(u8 *)(addr++); > if (v != 0x0f) > return BUG_NONE; > v = *(u8 *)(addr++); > if (v == 0x0b) > return BUG_UD2; > if (v != 0xb9) > return BUG_NONE; > > if (X86_MODRM_RM(v) == 4) > addr++; /* consume SiB */ > > *imm = 0; > if (X86_MODRM_MOD(v) == 1) > *imm = *(u8 *)addr; > if (X86_MORRM_MOD(v) == 2) > *imm = *(u32 *)addr; > > // WARN on MOD(v)==3 ?? > > return BUG_UD1; > }
Thanks for the example! (I think it should use macros instead of open-coded "0x67", "0x0f", etc, but yeah.) > Why does the thing emit the asop prefix at all through? afaict it > doesn't affect the immediate you want to get at. And if it does this > prefix, should we worry about other prefixes? Ideally we'd not accept > any prefixes. AFAICT it's because it's a small immediate? For an x86_64 build, this is how Clang is generating the UD1. -Kees -- Kees Cook