[PATCH] x86: add hintable NOPs emulation

[PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

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@orca.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 = &current->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;
+
+	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;
+#endif
+
 	do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
 		      ILL_ILLOPN, error_get_trap_addr(regs));
 }
-- 
2.34.1

Re: [PATCH] x86: add hintable NOPs emulation

[Peter Zijlstra]

On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives 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@orca.pet>

This is going to be terribly slow if there's a significant number of
traps (like with endbr32), but yeah, this ought to work.

One indenting fail below, other than that:

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  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 = &current->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)

That continuation wants to be aligned at (, not tab-width.

> +		return false;
> +
> +	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;
> +#endif
> +
>  	do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
>  		      ILL_ILLOPN, error_get_trap_addr(regs));
>  }
> -- 
> 2.34.1
> 

Re: [PATCH] x86: add hintable NOPs emulation

[Ahmed S. Darwish]

Hi Marcos,

On Wed, 20 Aug 2025, Marcos Del Sol Vives wrote:
...
>
> --- 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
> +
...
> --- 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
...
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> +
> +	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;
> +	}

Can we please remove all this 'hnop_warn' trickery?  Removing it will
simplifiy the code and avoid complicating 'thread_struct' further.

It's just the kernel doing its normal job.

And if the system is full of binaries with hintable NOPs, ratelimiting
will not save you much.  I got hit recently by a 'ratelimited'
correctible error PCI subsystem warning, and it still overflows the log
buffers of my Thinkpad laptop, in just 4 to 5 days :(

>
> static inline void handle_invalid_op(struct pt_regs *regs)
> {
> +#ifdef CONFIG_X86_HNOP_EMU
> +	if (user_mode(regs) && handle_hnop(regs))
> +		return;
> +#endif
> +
>

CPP conditionals within C function code are ugly.  Please do instead:

    static bool handle_hnop(struct pt_regs *regs)
    {
	if (!IS_ENABLED(CONFIG_X86_HNOP_EMU))
		return false;
	...
    }

Thanks for your contribution!

--
Ahmed S. Darwish
Linutronix GmbH

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

Hi Ahmed,

El 20/08/2025 a las 11:14, Ahmed S. Darwish escribió:
> Can we please remove all this 'hnop_warn' trickery?  Removing it will
> simplifiy the code and avoid complicating 'thread_struct' further.
> 
> It's just the kernel doing its normal job.
> 
> And if the system is full of binaries with hintable NOPs, ratelimiting
> will not save you much.  I got hit recently by a 'ratelimited'
> correctible error PCI subsystem warning, and it still overflows the log
> buffers of my Thinkpad laptop, in just 4 to 5 days :(

But I think the kernel should let the user know the binaries they're
running are having some performance penalty due to this emulation, in case
they want to recompile without the offending flags.

Without the logging, they'd be in the dark and might get confused on why
their programs are running slower than on other machines.

>>
>> static inline void handle_invalid_op(struct pt_regs *regs)
>> {
>> +#ifdef CONFIG_X86_HNOP_EMU
>> +	if (user_mode(regs) && handle_hnop(regs))
>> +		return;
>> +#endif
>> +
>>
> 
> CPP conditionals within C function code are ugly.  Please do instead:
> 
>     static bool handle_hnop(struct pt_regs *regs)
>     {
> 	if (!IS_ENABLED(CONFIG_X86_HNOP_EMU))
> 		return false;
> 	...
>     }
> 
> Thanks for your contribution!

I originally did that, but then realized it was not possible due to
"handle_hnop" depending on the conditionally-available "hnop_warn" flag.

Greetings,
Marcos

Re: [PATCH] x86: add hintable NOPs emulation

[Borislav Petkov]

On Wed, Aug 20, 2025 at 11:33:05AM +0200, Marcos Del Sol Vives wrote:
> But I think the kernel should let the user know the binaries they're
> running are having some performance penalty due to this emulation, in case
> they want to recompile without the offending flags.

Sure, once perhaps.

Do you want to let the user know for each binary?

And how many users do you really think will look at dmesg and recompile their
binaries?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 20/08/2025 a las 11:43, Borislav Petkov escribió:
> On Wed, Aug 20, 2025 at 11:33:05AM +0200, Marcos Del Sol Vives wrote:
>> But I think the kernel should let the user know the binaries they're
>> running are having some performance penalty due to this emulation, in case
>> they want to recompile without the offending flags.
> 
> Sure, once perhaps.
> 
> Do you want to let the user know for each binary?
> 
> And how many users do you really think will look at dmesg and recompile their
> binaries?

I mean, they should know what they need to recompile if they want to, not
just that their machine is having a bug triggered by some binary.

A global flag would mean they'd need to reboot to see if there is any other
binary triggering it too.

Whether they look or not at the dmesg I cannot tell, but if IOPL emulation
does logging this way, I assumed this should too. Otherwise, would that
mean IOPL emulation logging is also too verbose?

Re: [PATCH] x86: add hintable NOPs emulation

[Borislav Petkov]

On Wed, Aug 20, 2025 at 11:51:27AM +0200, Marcos Del Sol Vives wrote:
> I mean, they should know what they need to recompile if they want to, not
> just that their machine is having a bug triggered by some binary.

And what's stopping you from writing a proper error message explaining that?

And issuing that error message *exactly once* instead of flooding dmesg for no
good reason?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 20/08/2025 a las 11:55, Borislav Petkov escribió:
> On Wed, Aug 20, 2025 at 11:51:27AM +0200, Marcos Del Sol Vives wrote:
>> I mean, they should know what they need to recompile if they want to, not
>> just that their machine is having a bug triggered by some binary.
> 
> And what's stopping you from writing a proper error message explaining that?
> 
> And issuing that error message *exactly once* instead of flooding dmesg for no
> good reason?

Please define "once". Once per what? Per boot? Per executable? Per process?

Once per boot would mean they'd need to reboot to see if any other executables
are affected. Per executable AFAIK there are no facilities to do that, and the
closest is per process which is what it's currently being done (again, like
IOPL emulation which was already deemed okay a couple years ago and merged
into the kernel)

Re: [PATCH] x86: add hintable NOPs emulation

[Borislav Petkov]

On Wed, Aug 20, 2025 at 12:01:30PM +0200, Marcos Del Sol Vives wrote:
> Please define "once". Once per what? Per boot? Per executable? Per process?

pr_err_once(). Per boot.

> Once per boot would mean they'd need to reboot to see if any other executables
> are affected.

We'll cross that bridge when we get to it.

> (again, like IOPL emulation which was already deemed okay a couple years ago
> and merged into the kernel)

When it is flooding dmesg for no good reason, I wouldn't mind toning that down
too.

IOW, I wanna see a real, actual use case which justifies flooding dmesg. If
you don't have one, then we'll do it only when it is really warranted.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86: add hintable NOPs emulation

[Ahmed S. Darwish]

On Wed, 20 Aug 2025, Marcos Del Sol Vives wrote:
>
> But I think the kernel should let the user know the binaries they're
> running are having some performance penalty due to this emulation, in case
> they want to recompile without the offending flags.
>
> Without the logging, they'd be in the dark and might get confused on why
> their programs are running slower than on other machines.
>

Not convinced; especially all the extra 'thread_struct' noise.

>
> I originally did that, but then realized it was not possible due to
> "handle_hnop" depending on the conditionally-available "hnop_warn" flag.
>

Please do:

    #ifdef CONFIG_X86_HNOP_EMU
    static bool handle_hnop(struct pt_regs *regs)
    {
	// Reference 'hnop_warn' as much as you like
    }
    #else
    static bool handle_hnop(struct pt_regs *regs)
    {
	return false;
    }
    # endif

Then this ugliness:

    static inline void handle_invalid_op(struct pt_regs *regs)
    {
    #ifdef CONFIG_X86_HNOP_EMU
     	if (user_mode(regs) && handle_hnop(regs))
     		return;
    #endif
	...
    }

can become normal code:

    static inline void handle_invalid_op(struct pt_regs *regs)
    {
     	if (user_mode(regs) && handle_hnop(regs))
     		return;
	...
    }

Good luck,

--
Ahmed S. Darwish
Linutronix GmbH

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 20/08/2025 a las 12:08, Borislav Petkov escribió:
> On Wed, Aug 20, 2025 at 12:01:30PM +0200, Marcos Del Sol Vives wrote:
>> Please define "once". Once per what? Per boot? Per executable? Per process?
> 
> pr_err_once(). Per boot.

Would a simple:

> pr_warn_once("Your processor does not correctly handle hintable NOPs.\n");
> pr_warn_once("The kernel will emulate them, but the performance will be impacted!\n");

work for you, then? With no thread information, as that might make the user
think there is only once binary impacted.

Re: [PATCH] x86: add hintable NOPs emulation

[Borislav Petkov]

On Wed, Aug 20, 2025 at 12:21:40PM +0200, Marcos Del Sol Vives wrote:
> Would a simple:
> 
> > pr_warn_once("Your processor does not correctly handle hintable NOPs.\n");
> > pr_warn_once("The kernel will emulate them, but the performance will be impacted!\n");
> 
> work for you, then? With no thread information, as that might make the user
> think there is only once binary impacted.

I don't mind if you make the warning message as helpful as possible and even
dump current->comm and whatever else is needed to help the user address the
issue.

What I mind is flooding dmesg unnecessarily with the same stanzas over and
over again.

If the user can do something about it, then she should be able to find out
also which executables need to be recompiled.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86: add hintable NOPs emulation

[Ahmed S. Darwish]

On Wed, 20 Aug 2025, Ahmed S. Darwish wrote:
>
> Please do:
>
>     #ifdef CONFIG_X86_HNOP_EMU
>     static bool handle_hnop(struct pt_regs *regs)
>     {
> 	// Reference 'hnop_warn' as much as you like
>     }
>     #else
>     static bool handle_hnop(struct pt_regs *regs)
>     {
> 	return false;
>     }
>     # endif
>

And as previously suggested: remove the ugly hnop_warn stuff from
'thread_struct', then you can even just do:

    static bool handle_hnop(struct pt_regs *regs)
    {
	if (!IS_ENABLED(CONFIG_X86_HNOP_EMU))
		return false;

	pr_warn_once("%s[%d] Emulating hintable NOP\n"
		     "This warning will not be repeated; even for other binaries\n"
		     current->comm, task_pid_nr(current));

	...
    }

And everything else will fit quietly in place.

--
Ahmed S. Darwish
Linutronix GmbH

Re: [PATCH] x86: add hintable NOPs emulation

[H. Peter Anvin]

On August 19, 2025 6:34:46 PM PDT, Marcos Del Sol Vives <marcos@orca.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@orca.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 = &current->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;
>+
>+	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;
>+#endif
>+
> 	do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
> 		      ILL_ILLOPN, error_get_trap_addr(regs));
> }

Do those processors support FCOMI?

Re: [PATCH] x86: add hintable NOPs emulation

[Kees Cook]

On August 20, 2025 3:01:30 AM PDT, Marcos Del Sol Vives <marcos@orca.pet> wrote:
>El 20/08/2025 a las 11:55, Borislav Petkov escribió:
>> On Wed, Aug 20, 2025 at 11:51:27AM +0200, Marcos Del Sol Vives wrote:
>>> I mean, they should know what they need to recompile if they want to, not
>>> just that their machine is having a bug triggered by some binary.
>> 
>> And what's stopping you from writing a proper error message explaining that?
>> 
>> And issuing that error message *exactly once* instead of flooding dmesg for no
>> good reason?
>
>Please define "once". Once per what? Per boot? Per executable? Per process?
>
>Once per boot would mean they'd need to reboot to see if any other executables
>are affected. Per executable AFAIK there are no facilities to do that, and the
>closest is per process which is what it's currently being done (again, like
>IOPL emulation which was already deemed okay a couple years ago and merged
>into the kernel)

If they want to see them again, just: 

echo 1 >/sys/kernel/debug/clear_warn_once


-- 
Kees Cook

Re: [PATCH] x86: add hintable NOPs emulation

[H. Peter Anvin]

On August 19, 2025 6:34:46 PM PDT, Marcos Del Sol Vives <marcos@orca.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@orca.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 = &current->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;
>+
>+	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;
>+#endif
>+
> 	do_error_trap(regs, 0, "invalid opcode", X86_TRAP_UD, SIGILL,
> 		      ILL_ILLOPN, error_get_trap_addr(regs));
> }

Why are they using -fcf-protection=branch on 32 buts when the kernel vdso doesn't even (currently) support it?

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 21/08/2025 a las 3:43, H. Peter Anvin escribió:
> On August 19, 2025 6:34:46 PM PDT, Marcos Del Sol Vives <marcos@orca.pet> wrote:
>> 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.
>>
>> [1]: https://lists.debian.org/debian-devel/2023/10/msg00118.html
>> [2]: https://lore.kernel.org/all/20210626130313.1283485-1-marcos@orca.pet/
>
> Do those processors support FCOMI?

The Vortex86DX3, at the very least, does. Unlike the Vortex86MX I wrote the
previous patch for, this one seems to support all standard i686
instructions. It's modern enough to support SSE1, even.

I have tested it using the program I wrote for testing it on the -MX
last time: https://lore.kernel.org/all/b69e0c78-81eb-0d4d-dce5-076b5f239e28@orca.pet/.

> marcos@vdx3:~$ LANG=C gcc -Wall fucomi-test.c -o fucomi-test
> marcos@vdx3:~$ ./fucomi-test
> Float value: 6.210000
> FPU status: 3800 EFLAGS: 00000242

Which matches 1:1 my current desktop Ryzen 8645H.

Greetings,
Marcos

Re: [PATCH] x86: add hintable NOPs emulation

[David Laight]

On Wed, 20 Aug 2025 03:34:46 +0200
Marcos Del Sol Vives <marcos@orca.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@orca.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 = &current->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));
>  }

Re: [PATCH] x86: add hintable NOPs emulation

[David Laight]

On Wed, 20 Aug 2025 11:07:33 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives 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@orca.pet>  
> 
> This is going to be terribly slow if there's a significant number of
> traps (like with endbr32), but yeah, this ought to work.

Could you patch the memory resident page to contain a supported nop?
(without marking it 'dirty')
Then the same function wouldn't trap until the code page was reloaded
from the source file.

	David

Re: [PATCH] x86: add hintable NOPs emulation

[Peter Zijlstra]

On Thu, Aug 21, 2025 at 01:28:07PM +0100, David Laight wrote:
> On Wed, 20 Aug 2025 11:07:33 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives 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@orca.pet>  
> > 
> > This is going to be terribly slow if there's a significant number of
> > traps (like with endbr32), but yeah, this ought to work.
> 
> Could you patch the memory resident page to contain a supported nop?
> (without marking it 'dirty')
> Then the same function wouldn't trap until the code page was reloaded
> from the source file.

It would mean cloning the page as private. Yes you can do it, uprobes
has all the code for this. But it has non-trivial memory overhead.

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 21/08/2025 a las 14:26, David Laight escribió:
> Marcos Del Sol Vives <marcos@orca.pet> wrote:
>> +	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.
> 

Not really. The opcodes may have prefixes that are "removed" by the decode
function. ENDBR32 for instance is actually "F3 0F 1E FB", with a REP
prefix.

>> +#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.
> 

Other code I saw did this in the calling function itself (eg handle_bug)
so I did it here too.

Re: [PATCH] x86: add hintable NOPs emulation

[Peter Zijlstra]

On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives wrote:
> +static bool handle_hnop(struct pt_regs *regs)
> +{
> +	struct thread_struct *t = &current->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;

FWIW, you need to check for insn.opcode.nbytes == 2.

> +	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;
> +}

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 21/08/2025 a las 14:48, Peter Zijlstra escribió:
> On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives wrote:
>> +	/* 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;
> 
> FWIW, you need to check for insn.opcode.nbytes == 2.
> 

I can add it no problem for clarity, but would it be really necessary?

All opcodes in that range will have that length by the Intel SDM, so it seems
somewhat redundant, and if the opcode couldn't be read in full the decode
would've failed earlier.

insn_decode_mmio for example which I used as an example of software parsing
of instructions does not check any length if the prefix was 0x0f.

Re: [PATCH] x86: add hintable NOPs emulation

[Peter Zijlstra]

On Thu, Aug 21, 2025 at 03:45:29PM +0200, Marcos Del Sol Vives wrote:
> El 21/08/2025 a las 14:48, Peter Zijlstra escribió:
> > On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives wrote:
> >> +	/* 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;
> > 
> > FWIW, you need to check for insn.opcode.nbytes == 2.
> > 
> 
> I can add it no problem for clarity, but would it be really necessary?
> 
> All opcodes in that range will have that length by the Intel SDM, so it seems
> somewhat redundant, and if the opcode couldn't be read in full the decode
> would've failed earlier.
> 
> insn_decode_mmio for example which I used as an example of software parsing
> of instructions does not check any length if the prefix was 0x0f.

Yeah, I suppose you're right.

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 21/08/2025 a las 14:28, David Laight escribió:
>> This is going to be terribly slow if there's a significant number of
>> traps (like with endbr32), but yeah, this ought to work.
> 
> Could you patch the memory resident page to contain a supported nop?
> (without marking it 'dirty')
> Then the same function wouldn't trap until the code page was reloaded
> from the source file.
> 

While I had thought of that, to be honest I'm not knowledgeable enough
in kernel development to pull that off without being 100% sure I did not
introduce any compatibility or security issues, so I preferred for the
time being to play it safe.

Anyhow, I made a simple benchmark running "sudo" with NOPASSWD as provided
by Debian bookworm i686, and another version compiled without ENDBR32s, and
the overhead does not seem to be too high:

# time for i in {1..100}; do ./sudo-nocet echo "" >/dev/null; done
real    0m6,001s
user    0m3,664s
sys     0m1,576s

# time for i in {1..100}; do sudo echo "" >/dev/null; done
real    0m5,983s
user    0m3,546s
sys     0m1,717s

The original binary has 203 "endbr32"s, and they result in a 0.3% slowdown
for this binary in particular, which is totally acceptable IMO.

Greetings,
Marcos

Re: [PATCH] x86: add hintable NOPs emulation

[David Laight]

On Thu, 21 Aug 2025 14:46:59 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Aug 21, 2025 at 01:28:07PM +0100, David Laight wrote:
> > On Wed, 20 Aug 2025 11:07:33 +0200
> > Peter Zijlstra <peterz@infradead.org> wrote:
> >   
> > > On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives 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@orca.pet>    
> > > 
> > > This is going to be terribly slow if there's a significant number of
> > > traps (like with endbr32), but yeah, this ought to work.  
> > 
> > Could you patch the memory resident page to contain a supported nop?
> > (without marking it 'dirty')
> > Then the same function wouldn't trap until the code page was reloaded
> > from the source file.  
> 
> It would mean cloning the page as private. Yes you can do it, uprobes
> has all the code for this. But it has non-trivial memory overhead.

I was thinking it would be safe to do this change without cloning the page.
After all the change is needed for all processes executing the code.
That might only be easy on UP systems - but I doubt the affected CPU are SMP.

	David

Re: [PATCH] x86: add hintable NOPs emulation

[Marcos Del Sol Vives]

El 21/08/2025 a las 20:40, David Laight escribió:
> On Thu, 21 Aug 2025 14:46:59 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
>> It would mean cloning the page as private. Yes you can do it, uprobes
>> has all the code for this. But it has non-trivial memory overhead.
> 
> I was thinking it would be safe to do this change without cloning the page.
> After all the change is needed for all processes executing the code.
> That might only be easy on UP systems - but I doubt the affected CPU are SMP.
> 
> 	David
> 

Actually...

marcos@vdx3:~$ nproc
2
marcos@vdx3:~$ cat /proc/cpuinfo
processor       : 0
vendor_id       : Vortex86 SoC
cpu family      : 6
model           : 1
model name      : Vortex86DX3
stepping        : 1
cpu MHz         : 1000.017
physical id     : 0
siblings        : 1
core id         : 0
cpu cores       : 1
apicid          : 0
initial apicid  : 0
fdiv_bug        : no
f00f_bug        : no
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 3
wp              : yes
flags           : fpu pse tsc msr cx8 apic sep pge cmov mmx fxsr sse cpuid
bugs            : itlb_multihit
bogomips        : 2000.03
clflush size    : 32
cache_alignment : 32
address sizes   : 32 bits physical, 32 bits virtual
power management:

processor       : 1
vendor_id       : Vortex86 SoC
cpu family      : 6
model           : 1
model name      : Vortex86DX3
stepping        : 1
cpu MHz         : 1000.017
physical id     : 1
siblings        : 1
core id         : 0
cpu cores       : 1
apicid          : 1
initial apicid  : 1
fdiv_bug        : no
f00f_bug        : no
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 3
wp              : yes
flags           : fpu pse tsc msr cx8 apic sep pge cmov mmx fxsr sse cpuid
bugs            : itlb_multihit
bogomips        : 2000.03
clflush size    : 32
cache_alignment : 32
address sizes   : 32 bits physical, 32 bits virtual
power management:

Vortex SoCs are true oddballs, aren't they?

Re: [PATCH] x86: add hintable NOPs emulation

[H. Peter Anvin]

On August 21, 2025 5:48:04 AM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Wed, Aug 20, 2025 at 03:34:46AM +0200, Marcos Del Sol Vives wrote:
>> +static bool handle_hnop(struct pt_regs *regs)
>> +{
>> +	struct thread_struct *t = &current->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;
>
>FWIW, you need to check for insn.opcode.nbytes == 2.
>
>> +	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;
>> +}

No, hintable noops apply to any modr/m.