[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ea88e0d2-2ada-40e6-9bca-06a598dba70d@citrix.com>
Date: Tue, 5 Nov 2024 15:19:23 +0000
From: Andrew Cooper <andrew.cooper3@...rix.com>
To: Peter Zijlstra <peterz@...radead.org>, x86@...nel.org
Cc: linux-kernel@...r.kernel.org, alyssa.milburn@...el.com,
scott.d.constable@...el.com, joao@...rdrivepizza.com, jpoimboe@...nel.org,
alexei.starovoitov@...il.com, ebiggers@...nel.org, samitolvanen@...gle.com,
kees@...nel.org
Subject: Re: [PATCH 5/8] x86/traps: Cleanup and robustify decode_bug()
On 05/11/2024 11:39 am, Peter Zijlstra wrote:
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -110,24 +117,37 @@ __always_inline int decode_bug(unsigned
> return BUG_NONE;
>
> v = *(u8 *)(addr++);
> - if (v == SECOND_BYTE_OPCODE_UD2)
> + if (v == SECOND_BYTE_OPCODE_UD2) {
> + *len = addr - start;
> return BUG_UD2;
> + }
>
> - if (!IS_ENABLED(CONFIG_UBSAN_TRAP) || v != SECOND_BYTE_OPCODE_UD1)
> + if (v != SECOND_BYTE_OPCODE_UD1)
> return BUG_NONE;
>
> - /* Retrieve the immediate (type value) for the UBSAN UD1 */
> - v = *(u8 *)(addr++);
> - if (X86_MODRM_RM(v) == 4)
> - addr++;
> -
> *imm = 0;
> - if (X86_MODRM_MOD(v) == 1)
> - *imm = *(u8 *)addr;
> - else if (X86_MODRM_MOD(v) == 2)
> - *imm = *(u32 *)addr;
> - else
> - WARN_ONCE(1, "Unexpected MODRM_MOD: %u\n", X86_MODRM_MOD(v));
> + v = *(u8 *)(addr++); /* ModRM */
> +
> + /* Decode immediate, if present */
> + if (X86_MODRM_MOD(v) != 3) {
> + if (X86_MODRM_RM(v) == 4)
> + addr++; /* Skip SIB byte */
> +
> + if (X86_MODRM_MOD(v) == 1) {
> + *imm = *(s8 *)addr;
> + addr += 1;
> +
> + } else if (X86_MODRM_MOD(v) == 2) {
> + *imm = *(s32 *)addr;
> + addr += 4;
> + }
> + }
> +
> + /* record instruction length */
> + *len = addr - start;
`ud1 0(%rip),%eax` has something to say about this length calculation[1].
You need the Mod = 0, RM = 5 case wired into addr += 4 without filling
in imm.
~Andrew
[1] or maybe you've got something rude to say about those of us who
encode instructions like that...[2]
[2] It's perhaps fortunate that decode_bug() doesn't know what a REX
prefix is.
Powered by blists - more mailing lists