linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86 Handle trap flag when instruction is emulated
@ 2025-07-29 21:44 Samuele Cerea
  2025-08-22 17:01 ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Samuele Cerea @ 2025-07-29 21:44 UTC (permalink / raw)
  To: x86, linux-kernel; +Cc: tglx, mingo, bp, dave.hansen, Samuele Cerea

When the kernel emulates an instruction (for UIMP instruction emulation
and iopl emulation) the trap flag (TF) is currently ignored. As a result no
SIGTRAP event is delivered when the instruction is emulated successfully,
breaking the expected behavior for signle-stepping in debuggers.

This patch checks if the TF is set and sends the expected SIGTRAP signal
to the user. Other exception take precedence over the trap flag the SIGTRAP
signal is sent only if the emulation was successful.

The bug can be reproduced by signle-stepping in this code:
    nop
    sldt rax
    sldt rax
    nop
The two sldt instruction will be skipped an the debugger will step directly
to the second nop instruction.

Signed-off-by: Samuele Cerea <samuele@cerea.dev>
---
 arch/x86/kernel/traps.c |  9 +++++++++
 arch/x86/kernel/umip.c  | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 36354b470590..6e7d07a5f587 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -705,6 +705,15 @@ static bool fixup_iopl_exception(struct pt_regs *regs)
 	}
 
 	regs->ip += 1;
+
+	/* If the instruction was emulated successfully, emulate trap flag */
+	if (regs->flags & X86_EFLAGS_TF) {
+		t->cr2 = regs->ip;
+		t->trap_nr = X86_TRAP_DB;
+		t->error_code = 0;
+		force_sig_fault(SIGTRAP, TRAP_TRACE, (void __user *)regs->ip);
+	}
+
 	return true;
 }
 
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index 5a4b21389b1d..c4c462074f1d 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -342,6 +342,7 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	unsigned long *reg_addr;
 	void __user *uaddr;
 	struct insn insn;
+	struct task_struct *tsk = current;
 
 	if (!regs)
 		return false;
@@ -407,5 +408,14 @@ bool fixup_umip_exception(struct pt_regs *regs)
 
 	/* increase IP to let the program keep going */
 	regs->ip += insn.length;
+
+	/* If the instruction was emulated successfully, emulate trap flag */
+	if (regs->flags & X86_EFLAGS_TF) {
+		tsk->thread.cr2 = regs->ip;
+		tsk->thread.trap_nr = X86_TRAP_DB;
+		tsk->thread.error_code = 0;
+		force_sig_fault(SIGTRAP, TRAP_TRACE, (void __user *)regs->ip);
+	}
+
 	return true;
 }
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86 Handle trap flag when instruction is emulated
  2025-07-29 21:44 [PATCH] x86 Handle trap flag when instruction is emulated Samuele Cerea
@ 2025-08-22 17:01 ` Borislav Petkov
  2025-09-01 16:25   ` [PATCH v2] x86/traps: " Samuele Cerea
  0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2025-08-22 17:01 UTC (permalink / raw)
  To: Samuele Cerea; +Cc: x86, linux-kernel, tglx, mingo, dave.hansen

On Tue, Jul 29, 2025 at 11:44:33PM +0200, Samuele Cerea wrote:
> Subject: Re: [PATCH] x86 Handle trap flag when instruction is emulated

Add a proper subject prefix:

"x86/traps: Handle ..."

or so.

> When the kernel emulates an instruction (for UIMP instruction emulation
> and iopl emulation) the trap flag (TF) is currently ignored. As a result no
> SIGTRAP event is delivered when the instruction is emulated successfully,
> breaking the expected behavior for signle-stepping in debuggers.
> 
> This patch checks if the TF is set and sends the expected SIGTRAP signal

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

> to the user. Other exception take precedence over the trap flag the SIGTRAP
> signal is sent only if the emulation was successful.
> 
> The bug can be reproduced by signle-stepping in this code:
>     nop
>     sldt rax
>     sldt rax
>     nop
> The two sldt instruction will be skipped an the debugger will step directly
> to the second nop instruction.

Please spell all insns in all caps.

> Signed-off-by: Samuele Cerea <samuele@cerea.dev>
> ---
>  arch/x86/kernel/traps.c |  9 +++++++++
>  arch/x86/kernel/umip.c  | 10 ++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 36354b470590..6e7d07a5f587 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -705,6 +705,15 @@ static bool fixup_iopl_exception(struct pt_regs *regs)
>  	}
>  
>  	regs->ip += 1;
> +
> +	/* If the instruction was emulated successfully, emulate trap flag */
> +	if (regs->flags & X86_EFLAGS_TF) {
> +		t->cr2 = regs->ip;
> +		t->trap_nr = X86_TRAP_DB;
> +		t->error_code = 0;
> +		force_sig_fault(SIGTRAP, TRAP_TRACE, (void __user *)regs->ip);
> +	}

This looks like it wants to be a separate function instead of replicating the
same code below.

> +
>  	return true;
>  }
>  
> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
> index 5a4b21389b1d..c4c462074f1d 100644
> --- a/arch/x86/kernel/umip.c
> +++ b/arch/x86/kernel/umip.c
> @@ -342,6 +342,7 @@ bool fixup_umip_exception(struct pt_regs *regs)
>  	unsigned long *reg_addr;
>  	void __user *uaddr;
>  	struct insn insn;
> +	struct task_struct *tsk = current;

The tip-tree preferred ordering of variable declarations at the
beginning of a function is reverse fir tree order::

	struct long_struct_name *descriptive_name;
	unsigned long foo, bar;
	unsigned int tmp;
	int ret;

The above is faster to parse than the reverse ordering::

	int ret;
	unsigned int tmp;
	unsigned long foo, bar;
	struct long_struct_name *descriptive_name;

And even more so than random ordering::

	unsigned long foo, bar;
	int ret;
	struct long_struct_name *descriptive_name;
	unsigned int tmp;

>  
>  	if (!regs)
>  		return false;
> @@ -407,5 +408,14 @@ bool fixup_umip_exception(struct pt_regs *regs)
>  
>  	/* increase IP to let the program keep going */
>  	regs->ip += insn.length;
> +
> +	/* If the instruction was emulated successfully, emulate trap flag */
> +	if (regs->flags & X86_EFLAGS_TF) {
> +		tsk->thread.cr2 = regs->ip;
> +		tsk->thread.trap_nr = X86_TRAP_DB;
> +		tsk->thread.error_code = 0;
> +		force_sig_fault(SIGTRAP, TRAP_TRACE, (void __user *)regs->ip);
> +	}
> +
>  	return true;
>  }
> -- 

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] x86/traps: Handle trap flag when instruction is emulated
  2025-08-22 17:01 ` Borislav Petkov
@ 2025-09-01 16:25   ` Samuele Cerea
  2025-09-01 18:26     ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Samuele Cerea @ 2025-09-01 16:25 UTC (permalink / raw)
  To: bp; +Cc: x86, linux-kernel, tglx, mingo, dave.hansen, Samuele Cerea

Simulate the trap flag (TF) behavior when the kernel emulates UIMP
instructions and iopl instructions.

When an instruction is emulated successfully and the TF is set, send a
SIGTRAP signal to the process, as it would happen for a normally
executed instruction.

Fix a problem with debuggers when signle-stepping instructions where
emulated instruction would get skipped.

Here is an example of the problem:
    NOP
    SLDT %rax
    SLDT %rax
    NOP
The two SLDT instructions will be skipped an the debugger will step
directly to the second NOP instruction.

Signed-off-by: Samuele Cerea <samuele@cerea.dev>
---
I fixed the issues you pointed out, hopefully now everything is correct

 arch/x86/include/asm/traps.h |  2 ++
 arch/x86/kernel/traps.c      | 14 ++++++++++++++
 arch/x86/kernel/umip.c       |  1 +
 3 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 869b88061801..7742a6d05158 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -39,6 +39,8 @@ void math_emulate(struct math_emu_info *);
 
 bool fault_in_kernel_space(unsigned long address);
 
+void emulate_trap_flag(struct pt_regs *regs);
+
 #ifdef CONFIG_VMAP_STACK
 void __noreturn handle_stack_overflow(struct pt_regs *regs,
 				      unsigned long fault_address,
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 36354b470590..bea28473866b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -678,6 +678,19 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs,
 
 #define GPFSTR "general protection fault"
 
+void emulate_trap_flag(struct pt_regs *regs)
+{
+	struct task_struct *tsk = current;
+
+	/* If the instruction was emulated successfully, emulate trap flag */
+	if (regs->flags & X86_EFLAGS_TF) {
+		tsk->thread.cr2 = regs->ip;
+		tsk->thread.trap_nr = X86_TRAP_DB;
+		tsk->thread.error_code = 0;
+		force_sig_fault(SIGTRAP, TRAP_TRACE, (void __user *)regs->ip);
+	}
+}
+
 static bool fixup_iopl_exception(struct pt_regs *regs)
 {
 	struct thread_struct *t = &current->thread;
@@ -705,6 +718,7 @@ static bool fixup_iopl_exception(struct pt_regs *regs)
 	}
 
 	regs->ip += 1;
+	emulate_trap_flag(regs);
 	return true;
 }
 
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index 5a4b21389b1d..8a5f33562bb4 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -407,5 +407,6 @@ bool fixup_umip_exception(struct pt_regs *regs)
 
 	/* increase IP to let the program keep going */
 	regs->ip += insn.length;
+	emulate_trap_flag(regs);
 	return true;
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] x86/traps: Handle trap flag when instruction is emulated
  2025-09-01 16:25   ` [PATCH v2] x86/traps: " Samuele Cerea
@ 2025-09-01 18:26     ` Andrew Cooper
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2025-09-01 18:26 UTC (permalink / raw)
  To: samuele; +Cc: bp, dave.hansen, linux-kernel, mingo, tglx, x86

> Simulate the trap flag (TF) behavior when the kernel emulates UIMP
> instructions and iopl instructions.

You'll want to do this for CPUID too when CPUID Faulting is active.

> diff
> <https://lore.kernel.org/lkml/20250901162527.18247-2-samuele@cerea.dev/#iZ31arch:x86:kernel:traps.c>
> --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index
> 36354b470590..bea28473866b 100644 --- a/arch/x86/kernel/traps.c +++
> b/arch/x86/kernel/traps.c
> @@ -705,6 +718,7 @@ static bool fixup_iopl_exception(struct pt_regs
> *regs)  	}
>  
>  	regs->ip += 1;
> + emulate_trap_flag(regs);  	return true;
>  }
>  

There's a fun bug in fixup_iopl_exception() which you're turning from
latent to real.

Not all STI/CLI instructions are 1 byte long.  If they have a redundant
prefix, the old logic would simply brute-force through the instruction 1
prefix byte at a time.

But now, you'll generate SIGTRAP in the middle of the instruction.

~Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-01 18:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-29 21:44 [PATCH] x86 Handle trap flag when instruction is emulated Samuele Cerea
2025-08-22 17:01 ` Borislav Petkov
2025-09-01 16:25   ` [PATCH v2] x86/traps: " Samuele Cerea
2025-09-01 18:26     ` Andrew Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).