linux-kernel - Re: [PATCH] x86 Handle trap flag when instruction is emulated

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250822170107.GGaKiiU-HFkxnbymhY@fat_crate.local>
Date: Fri, 22 Aug 2025 19:01:07 +0200
From: Borislav Petkov <bp@...en8.de>
To: Samuele Cerea <samuele@...ea.dev>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, tglx@...utronix.de,
	mingo@...hat.com, dave.hansen@...ux.intel.com
Subject: Re: [PATCH] x86 Handle trap flag when instruction is emulated

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@...ea.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