[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250821132605.2093c37a@pumpkin>
Date: Thu, 21 Aug 2025 13:26:05 +0100
From: David Laight <david.laight.linux@...il.com>
To: Marcos Del Sol Vives <marcos@...a.pet>
Cc: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, Ingo
Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen
<dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin"
<hpa@...or.com>, Brian Gerst <brgerst@...il.com>, Uros Bizjak
<ubizjak@...il.com>, Ard Biesheuvel <ardb@...nel.org>, David Kaplan
<david.kaplan@....com>, "Ahmed S. Darwish" <darwi@...utronix.de>, Kees Cook
<kees@...nel.org>, "Peter Zijlstra (Intel)" <peterz@...radead.org>, Andrew
Cooper <andrew.cooper3@...rix.com>, Oleg Nesterov <oleg@...hat.com>, "Xin
Li (Intel)" <xin@...or.com>, Sabyrzhan Tasbolatov <snovitoll@...il.com>
Subject: Re: [PATCH] x86: add hintable NOPs emulation
On Wed, 20 Aug 2025 03:34:46 +0200
Marcos Del Sol Vives <marcos@...a.pet> wrote:
> Hintable NOPs are a series of instructions introduced by Intel with the
> Pentium Pro (i686), and described in US patent US5701442A.
>
> These instructions were reserved to allow backwards-compatible changes
> in the instruction set possible, by having old processors treat them as
> variable-length NOPs, while having other semantics in modern processors.
>
> Some modern uses are:
> - Multi-byte/long NOPs
> - Indirect Branch Tracking (ENDBR32)
> - Shadow Stack (part of CET)
>
> Some processors advertising i686 compatibility lack full support for
> them, which may cause #UD to be incorrectly triggered, crashing software
> that uses then with an unexpected SIGILL.
>
> One such software is sudo in Debian bookworm, which is compiled with
> GCC -fcf-protection=branch and contains ENDBR32 instructions. It crashes
> on my Vortex86DX3 processor and VIA C3 Nehalem processors [1].
>
> This patch is a much simplified version of my previous patch for x86
> instruction emulation [2], that only emulates hintable NOPs.
>
> When #UD is raised, it checks if the opcode corresponds to a hintable NOP
> in user space. If true, it warns the user via the dmesg and advances the
> instruction pointer, thus emulating its expected NOP behaviour.
>
> [1]: https://lists.debian.org/debian-devel/2023/10/msg00118.html
> [2]: https://lore.kernel.org/all/20210626130313.1283485-1-marcos@orca.pet/
>
> Signed-off-by: Marcos Del Sol Vives <marcos@...a.pet>
> ---
> arch/x86/Kconfig | 29 +++++++++++++++++++++++++
> arch/x86/include/asm/processor.h | 4 ++++
> arch/x86/kernel/process.c | 3 +++
> arch/x86/kernel/traps.c | 36 ++++++++++++++++++++++++++++++++
> 4 files changed, 72 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 58d890fe2100..a6daebdc2573 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1286,6 +1286,35 @@ config X86_IOPL_IOPERM
> ability to disable interrupts from user space which would be
> granted if the hardware IOPL mechanism would be used.
>
> +config X86_HNOP_EMU
> + bool "Hintable NOPs emulation"
> + depends on X86_32
> + default y
> + help
> + Hintable NOPs are a series of instructions introduced by Intel with
> + the Pentium Pro (i686), and described in US patent US5701442A.
> +
> + These instructions were reserved to allow backwards-compatible
> + changes in the instruction set possible, by having old processors
> + treat them as variable-length NOPs, while having other semantics in
> + modern processors.
> +
> + Some modern uses are:
> + - Multi-byte/long NOPs
> + - Indirect Branch Tracking (ENDBR32)
> + - Shadow Stack (part of CET)
> +
> + Some processors advertising i686 compatibility (such as Cyrix MII,
> + VIA C3 Nehalem or DM&P Vortex86DX3) lack full support for them,
> + which may cause SIGILL to be incorrectly raised in user space when
> + a hintable NOP is encountered.
> +
> + Say Y here if you want the kernel to emulate them, allowing programs
> + that make use of them to run transparently on such processors.
> +
> + This emulation has no performance penalty for processors that
> + properly support them, so if unsure, enable it.
> +
> config TOSHIBA
> tristate "Toshiba Laptop support"
> depends on X86_32
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index bde58f6510ac..c34fb678c4de 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -499,6 +499,10 @@ struct thread_struct {
>
> unsigned int iopl_warn:1;
>
> +#ifdef CONFIG_X86_HNOP_EMU
> + unsigned int hnop_warn:1;
> +#endif
> +
> /*
> * Protection Keys Register for Userspace. Loaded immediately on
> * context switch. Store it in thread_struct to avoid a lookup in
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 1b7960cf6eb0..6ec8021638d0 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -178,6 +178,9 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
> p->thread.io_bitmap = NULL;
> clear_tsk_thread_flag(p, TIF_IO_BITMAP);
> p->thread.iopl_warn = 0;
> +#ifdef CONFIG_X86_HNOP_EMU
> + p->thread.hnop_warn = 0;
> +#endif
> memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
>
> #ifdef CONFIG_X86_64
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 36354b470590..2dcb7d7edf8a 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -295,12 +295,48 @@ DEFINE_IDTENTRY(exc_overflow)
> do_error_trap(regs, 0, "overflow", X86_TRAP_OF, SIGSEGV, 0, NULL);
> }
>
> +#ifdef CONFIG_X86_HNOP_EMU
> +static bool handle_hnop(struct pt_regs *regs)
> +{
> + struct thread_struct *t = ¤t->thread;
> + unsigned char buf[MAX_INSN_SIZE];
> + unsigned long nr_copied;
> + struct insn insn;
> +
> + nr_copied = insn_fetch_from_user(regs, buf);
> + if (nr_copied <= 0)
> + return false;
> +
> + if (!insn_decode_from_regs(&insn, regs, buf, nr_copied))
> + return false;
> +
> + /* Hintable NOPs cover 0F 18 to 0F 1F */
> + if (insn.opcode.bytes[0] != 0x0F ||
> + insn.opcode.bytes[1] < 0x18 || insn.opcode.bytes[1] > 0x1F)
> + return false;
Can you swap the order of those tests?
Looks like the 'decode' is only needed for the length.
> +
> + if (!t->hnop_warn) {
> + pr_warn_ratelimited("%s[%d] emulating hintable NOP, ip:%lx\n",
> + current->comm, task_pid_nr(current), regs->ip);
> + t->hnop_warn = 1;
> + }
> +
> + regs->ip += insn.length;
> + return true;
> +}
> +#endif
> +
> #ifdef CONFIG_X86_F00F_BUG
> void handle_invalid_op(struct pt_regs *regs)
> #else
> static inline void handle_invalid_op(struct pt_regs *regs)
> #endif
> {
> +#ifdef CONFIG_X86_HNOP_EMU
> + if (user_mode(regs) && handle_hnop(regs))
> + return;
Why not move the user_mode() test into handle_hnop() ?
Should make the config tests easier.
David
> +#endif
> +
> do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
> ILL_ILLOPN, error_get_trap_addr(regs));
> }
Powered by blists - more mailing lists