[PATCH v4 0/2] x86/fred: Prevent single-step upon ERETU completion

[PATCH v4 0/2] x86/fred: Prevent single-step upon ERETU completion

[Xin Li (Intel)]

IDT event delivery has a debug hole in which it does not generate #DB
upon returning to userspace before the first userspace instruction is
executed if the Trap Flag (TF) is set.

FRED closes this hole by introducing a software event flag, i.e., bit
17 of the augmented SS: if the bit is set and ERETU would result in
RFLAGS.TF = 1, a single-step trap will be pending upon completion of
ERETU.

However I overlooked properly setting and clearing the bit in different
situations.  Thus when FRED is enabled, if the Trap Flag (TF) is set
without an external debugger attached, it can lead to an infinite loop
in the SIGTRAP handler.  To avoid this, the software event flag in the
augmented SS must be cleared, ensuring that no single-step trap remains
pending when ERETU completes.

This patch set combines the fix [1] and its corresponding selftest [2]
(requested by Dave Hansen) into one patch set.

[1] https://lore.kernel.org/lkml/20250523050153.3308237-1-xin@zytor.com/
[2] https://lore.kernel.org/lkml/20250530230707.2528916-1-xin@zytor.com/


This patch set is based on tip/x86/urgent branch as of today.


Xin Li (Intel) (2):
  x86/fred/signal: Prevent single-step upon ERETU completion
  selftests/x86: Add a test to detect infinite sigtrap handler loop

 arch/x86/include/asm/sighandling.h         | 22 +++++
 arch/x86/kernel/signal_32.c                |  4 +
 arch/x86/kernel/signal_64.c                |  4 +
 tools/testing/selftests/x86/Makefile       |  2 +-
 tools/testing/selftests/x86/sigtrap_loop.c | 97 ++++++++++++++++++++++
 5 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/sigtrap_loop.c


base-commit: dd2922dcfaa3296846265e113309e5f7f138839f
-- 
2.49.0

[PATCH v4 1/2] x86/fred/signal: Prevent single-step upon ERETU completion

[Xin Li (Intel)]

Clear the software event flag in the augmented SS to prevent infinite
SIGTRAP handler loop if the trap flag (TF) is set without an external
debugger attached.

Following is a typical single-stepping flow for a user process:

1) The user process is prepared for single-stepping by setting
   RFLAGS.TF = 1.
2) When any instruction in user space completes, a #DB is triggered.
3) The kernel handles the #DB and returns to user space, invoking the
   SIGTRAP handler with RFLAGS.TF = 0.
4) After the SIGTRAP handler finishes, the user process performs a
   sigreturn syscall, restoring the original state, including
   RFLAGS.TF = 1.
5) Goto step 2.

According to the FRED specification:

A) Bit 17 in the augmented SS is designated as the software event
   flag, which is set to 1 for FRED event delivery of SYSCALL,
   SYSENTER, or INT n.
B) If bit 17 of the augmented SS is 1 and ERETU would result in
   RFLAGS.TF = 1, a single-step trap will be pending upon completion
   of ERETU.

In step 4) above, the software event flag is set upon the sigreturn
syscall, and its corresponding ERETU would restore RFLAGS.TF = 1.
This combination causes a pending single-step trap upon completion of
ERETU.  Therefore, another #DB is triggered before any user space
instruction is executed, which leads to an infinite loop in which the
SIGTRAP handler keeps being invoked on the same user space IP.

Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
Cc: stable@vger.kernel.org
---

Change in v3:
*) Use "#ifdef CONFIG_X86_FRED" instead of IS_ENABLED(CONFIG_X86_FRED)
   (Intel LKP).

Change in v2:
*) Remove the check cpu_feature_enabled(X86_FEATURE_FRED), because
   regs->fred_ss.swevent will always be 0 otherwise (H. Peter Anvin).
---
 arch/x86/include/asm/sighandling.h | 22 ++++++++++++++++++++++
 arch/x86/kernel/signal_32.c        |  4 ++++
 arch/x86/kernel/signal_64.c        |  4 ++++
 3 files changed, 30 insertions(+)

diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index e770c4fc47f4..b8481d33ba8e 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -24,4 +24,26 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
 
+/*
+ * To prevent infinite SIGTRAP handler loop if the trap flag (TF) is set
+ * without an external debugger attached, clear the software event flag in
+ * the augmented SS, ensuring no single-step trap is pending upon ERETU
+ * completion.
+ *
+ * Note, this function should be called in sigreturn() before the original
+ * state is restored to make sure the TF is read from the entry frame.
+ */
+static __always_inline void prevent_single_step_upon_eretu(struct pt_regs *regs)
+{
+	/*
+	 * If the trap flag (TF) is set, i.e., the sigreturn() SYSCALL instruction
+	 * is being single-stepped, do not clear the software event flag in the
+	 * augmented SS, thus a debugger won't skip over the following instruction.
+	 */
+#ifdef CONFIG_X86_FRED
+	if (!(regs->flags & X86_EFLAGS_TF))
+		regs->fred_ss.swevent = 0;
+#endif
+}
+
 #endif /* _ASM_X86_SIGHANDLING_H */
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index 98123ff10506..42bbc42bd350 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -152,6 +152,8 @@ SYSCALL32_DEFINE0(sigreturn)
 	struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8);
 	sigset_t set;
 
+	prevent_single_step_upon_eretu(regs);
+
 	if (!access_ok(frame, sizeof(*frame)))
 		goto badframe;
 	if (__get_user(set.sig[0], &frame->sc.oldmask)
@@ -175,6 +177,8 @@ SYSCALL32_DEFINE0(rt_sigreturn)
 	struct rt_sigframe_ia32 __user *frame;
 	sigset_t set;
 
+	prevent_single_step_upon_eretu(regs);
+
 	frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
 
 	if (!access_ok(frame, sizeof(*frame)))
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index ee9453891901..d483b585c6c6 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -250,6 +250,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
 	sigset_t set;
 	unsigned long uc_flags;
 
+	prevent_single_step_upon_eretu(regs);
+
 	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
 	if (!access_ok(frame, sizeof(*frame)))
 		goto badframe;
@@ -366,6 +368,8 @@ COMPAT_SYSCALL_DEFINE0(x32_rt_sigreturn)
 	sigset_t set;
 	unsigned long uc_flags;
 
+	prevent_single_step_upon_eretu(regs);
+
 	frame = (struct rt_sigframe_x32 __user *)(regs->sp - 8);
 
 	if (!access_ok(frame, sizeof(*frame)))
-- 
2.49.0

[PATCH v4 2/2] selftests/x86: Add a test to detect infinite sigtrap handler loop

[Xin Li (Intel)]

When FRED is enabled, if the Trap Flag (TF) is set without an external
debugger attached, it can lead to an infinite loop in the SIGTRAP
handler.  To avoid this, the software event flag in the augmented SS
must be cleared, ensuring that no single-step trap remains pending when
ERETU completes.

This test checks for that specific scenario—verifying whether the kernel
correctly prevents an infinite SIGTRAP loop in this edge case when FRED
is enabled.

The test should _always_ pass with IDT event delivery, thus no need to
disable the test even when FRED is not enabled.

Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---

Changes in this version:
*) Address review comments from Sohil.
---
 tools/testing/selftests/x86/Makefile       |  2 +-
 tools/testing/selftests/x86/sigtrap_loop.c | 97 ++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/x86/sigtrap_loop.c

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index f703fcfe9f7c..83148875a12c 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -12,7 +12,7 @@ CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh "$(CC)" trivial_program.c -no-pie)
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \
 			check_initial_reg_state sigreturn iopl ioperm \
-			test_vsyscall mov_ss_trap \
+			test_vsyscall mov_ss_trap sigtrap_loop \
 			syscall_arg_fault fsgsbase_restore sigaltstack
 TARGETS_C_BOTHBITS += nx_stack
 TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \
diff --git a/tools/testing/selftests/x86/sigtrap_loop.c b/tools/testing/selftests/x86/sigtrap_loop.c
new file mode 100644
index 000000000000..dfb05769551d
--- /dev/null
+++ b/tools/testing/selftests/x86/sigtrap_loop.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Intel Corporation
+ */
+#define _GNU_SOURCE
+
+#include <err.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ucontext.h>
+
+#ifdef __x86_64__
+# define REG_IP REG_RIP
+#else
+# define REG_IP REG_EIP
+#endif
+
+static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), int flags)
+{
+	struct sigaction sa;
+
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_sigaction = handler;
+	sa.sa_flags = SA_SIGINFO | flags;
+	sigemptyset(&sa.sa_mask);
+
+	if (sigaction(sig, &sa, 0))
+		err(1, "sigaction");
+
+	return;
+}
+
+static unsigned int loop_count_on_same_ip;
+
+static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
+{
+	ucontext_t *ctx = (ucontext_t *)ctx_void;
+	static unsigned long last_trap_ip;
+
+	if (last_trap_ip == ctx->uc_mcontext.gregs[REG_IP]) {
+		printf("\tTrapped at %016lx\n", last_trap_ip);
+
+		/*
+		 * If the same IP is hit more than 10 times in a row, it is
+		 * _considered_ an infinite loop.
+		 */
+		if (++loop_count_on_same_ip > 10) {
+			printf("[FAIL]\tDetected sigtrap infinite loop\n");
+			exit(1);
+		}
+
+		return;
+	}
+
+	loop_count_on_same_ip = 0;
+	last_trap_ip = ctx->uc_mcontext.gregs[REG_IP];
+	printf("\tTrapped at %016lx\n", last_trap_ip);
+}
+
+int main(int argc, char *argv[])
+{
+	sethandler(SIGTRAP, sigtrap, 0);
+
+	/*
+	 * Set the Trap Flag (TF) to single-step the test code, therefore to
+	 * trigger a SIGTRAP signal after each instruction until the TF is
+	 * cleared.
+	 *
+	 * Because the arithmetic flags are not significant here, the TF is
+	 * set by pushing 0x302 onto the stack and then popping it into the
+	 * flags register.
+	 *
+	 * Four instructions in the following asm code are executed with the
+	 * TF set, thus the SIGTRAP handler is expected to run four times.
+	 */
+	printf("[RUN]\tsigtrap infinite loop detection\n");
+	asm volatile(
+#ifdef __x86_64__
+		/* Avoid clobbering the redzone */
+		"sub $128, %rsp\n\t"
+#endif
+		"push $0x302\n\t"
+		"popf\n\t"
+		"nop\n\t"
+		"nop\n\t"
+		"push $0x202\n\t"
+		"popf\n\t"
+#ifdef __x86_64__
+		"add $128, %rsp\n\t"
+#endif
+	);
+
+	printf("[OK]\tNo sigtrap infinite loop detected\n");
+	return 0;
+}
-- 
2.49.0

Re: [PATCH v4 1/2] x86/fred/signal: Prevent single-step upon ERETU completion

[H. Peter Anvin]

On June 5, 2025 11:10:19 AM PDT, "Xin Li (Intel)" <xin@zytor.com> wrote:
>Clear the software event flag in the augmented SS to prevent infinite
>SIGTRAP handler loop if the trap flag (TF) is set without an external
>debugger attached.
>
>Following is a typical single-stepping flow for a user process:
>
>1) The user process is prepared for single-stepping by setting
>   RFLAGS.TF = 1.
>2) When any instruction in user space completes, a #DB is triggered.
>3) The kernel handles the #DB and returns to user space, invoking the
>   SIGTRAP handler with RFLAGS.TF = 0.
>4) After the SIGTRAP handler finishes, the user process performs a
>   sigreturn syscall, restoring the original state, including
>   RFLAGS.TF = 1.
>5) Goto step 2.
>
>According to the FRED specification:
>
>A) Bit 17 in the augmented SS is designated as the software event
>   flag, which is set to 1 for FRED event delivery of SYSCALL,
>   SYSENTER, or INT n.
>B) If bit 17 of the augmented SS is 1 and ERETU would result in
>   RFLAGS.TF = 1, a single-step trap will be pending upon completion
>   of ERETU.
>
>In step 4) above, the software event flag is set upon the sigreturn
>syscall, and its corresponding ERETU would restore RFLAGS.TF = 1.
>This combination causes a pending single-step trap upon completion of
>ERETU.  Therefore, another #DB is triggered before any user space
>instruction is executed, which leads to an infinite loop in which the
>SIGTRAP handler keeps being invoked on the same user space IP.
>
>Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
>Signed-off-by: Xin Li (Intel) <xin@zytor.com>
>Cc: stable@vger.kernel.org
>---
>
>Change in v3:
>*) Use "#ifdef CONFIG_X86_FRED" instead of IS_ENABLED(CONFIG_X86_FRED)
>   (Intel LKP).
>
>Change in v2:
>*) Remove the check cpu_feature_enabled(X86_FEATURE_FRED), because
>   regs->fred_ss.swevent will always be 0 otherwise (H. Peter Anvin).
>---
> arch/x86/include/asm/sighandling.h | 22 ++++++++++++++++++++++
> arch/x86/kernel/signal_32.c        |  4 ++++
> arch/x86/kernel/signal_64.c        |  4 ++++
> 3 files changed, 30 insertions(+)
>
>diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
>index e770c4fc47f4..b8481d33ba8e 100644
>--- a/arch/x86/include/asm/sighandling.h
>+++ b/arch/x86/include/asm/sighandling.h
>@@ -24,4 +24,26 @@ int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
> int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
> int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
> 
>+/*
>+ * To prevent infinite SIGTRAP handler loop if the trap flag (TF) is set
>+ * without an external debugger attached, clear the software event flag in
>+ * the augmented SS, ensuring no single-step trap is pending upon ERETU
>+ * completion.
>+ *
>+ * Note, this function should be called in sigreturn() before the original
>+ * state is restored to make sure the TF is read from the entry frame.
>+ */
>+static __always_inline void prevent_single_step_upon_eretu(struct pt_regs *regs)
>+{
>+	/*
>+	 * If the trap flag (TF) is set, i.e., the sigreturn() SYSCALL instruction
>+	 * is being single-stepped, do not clear the software event flag in the
>+	 * augmented SS, thus a debugger won't skip over the following instruction.
>+	 */
>+#ifdef CONFIG_X86_FRED
>+	if (!(regs->flags & X86_EFLAGS_TF))
>+		regs->fred_ss.swevent = 0;
>+#endif
>+}
>+
> #endif /* _ASM_X86_SIGHANDLING_H */
>diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
>index 98123ff10506..42bbc42bd350 100644
>--- a/arch/x86/kernel/signal_32.c
>+++ b/arch/x86/kernel/signal_32.c
>@@ -152,6 +152,8 @@ SYSCALL32_DEFINE0(sigreturn)
> 	struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user *)(regs->sp-8);
> 	sigset_t set;
> 
>+	prevent_single_step_upon_eretu(regs);
>+
> 	if (!access_ok(frame, sizeof(*frame)))
> 		goto badframe;
> 	if (__get_user(set.sig[0], &frame->sc.oldmask)
>@@ -175,6 +177,8 @@ SYSCALL32_DEFINE0(rt_sigreturn)
> 	struct rt_sigframe_ia32 __user *frame;
> 	sigset_t set;
> 
>+	prevent_single_step_upon_eretu(regs);
>+
> 	frame = (struct rt_sigframe_ia32 __user *)(regs->sp - 4);
> 
> 	if (!access_ok(frame, sizeof(*frame)))
>diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
>index ee9453891901..d483b585c6c6 100644
>--- a/arch/x86/kernel/signal_64.c
>+++ b/arch/x86/kernel/signal_64.c
>@@ -250,6 +250,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
> 	sigset_t set;
> 	unsigned long uc_flags;
> 
>+	prevent_single_step_upon_eretu(regs);
>+
> 	frame = (struct rt_sigframe __user *)(regs->sp - sizeof(long));
> 	if (!access_ok(frame, sizeof(*frame)))
> 		goto badframe;
>@@ -366,6 +368,8 @@ COMPAT_SYSCALL_DEFINE0(x32_rt_sigreturn)
> 	sigset_t set;
> 	unsigned long uc_flags;
> 
>+	prevent_single_step_upon_eretu(regs);
>+
> 	frame = (struct rt_sigframe_x32 __user *)(regs->sp - 8);
> 
> 	if (!access_ok(frame, sizeof(*frame)))

Nitpick: "Prevent immediate repeat of single step trap on return from SIGTRAP handler"

Re: [PATCH v4 0/2] x86/fred: Prevent single-step upon ERETU completion

[Sohil Mehta]

On 6/5/2025 11:10 AM, Xin Li (Intel) wrote:

> 
> Xin Li (Intel) (2):
>   x86/fred/signal: Prevent single-step upon ERETU completion
>   selftests/x86: Add a test to detect infinite sigtrap handler loop
> 

I tested the patches on a machine that supports FRED. The results are as
expected:

FRED on  — no Fix   => FAIL
FRED off — no Fix   => PASS
FRED on  — with Fix => PASS
FRED off — with Fix => PASS

Tested-by: Sohil Mehta <sohil.mehta@intel.com>

Though, I haven't tested the 32-bit version of the selftest.

>  arch/x86/include/asm/sighandling.h         | 22 +++++
>  arch/x86/kernel/signal_32.c                |  4 +
>  arch/x86/kernel/signal_64.c                |  4 +
>  tools/testing/selftests/x86/Makefile       |  2 +-
>  tools/testing/selftests/x86/sigtrap_loop.c | 97 ++++++++++++++++++++++
>  5 files changed, 128 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/x86/sigtrap_loop.c
> 

Re: [PATCH v4 0/2] x86/fred: Prevent single-step upon ERETU completion

[Sohil Mehta]

On 6/5/2025 12:32 PM, Sohil Mehta wrote:
> On 6/5/2025 11:10 AM, Xin Li (Intel) wrote:
> 
>>
>> Xin Li (Intel) (2):
>>   x86/fred/signal: Prevent single-step upon ERETU completion
>>   selftests/x86: Add a test to detect infinite sigtrap handler loop
>>
> 
> I tested the patches on a machine that supports FRED. The results are as
> expected:
> 

Seeing a split lock warning when running the test:
x86/split lock detection: #DB: sigtrap_loop_64/4614 took a bus_lock trap
at address: 0x4011ae

Wanted to get this out sooner for awareness. Will figure out more
details and send out an update.

> FRED on  — no Fix   => FAIL
> FRED off — no Fix   => PASS
> FRED on  — with Fix => PASS
> FRED off — with Fix => PASS
> 
> Tested-by: Sohil Mehta <sohil.mehta@intel.com>
> 
> Though, I haven't tested the 32-bit version of the selftest.
> 
>>  arch/x86/include/asm/sighandling.h         | 22 +++++
>>  arch/x86/kernel/signal_32.c                |  4 +
>>  arch/x86/kernel/signal_64.c                |  4 +
>>  tools/testing/selftests/x86/Makefile       |  2 +-
>>  tools/testing/selftests/x86/sigtrap_loop.c | 97 ++++++++++++++++++++++
>>  5 files changed, 128 insertions(+), 1 deletion(-)
>>  create mode 100644 tools/testing/selftests/x86/sigtrap_loop.c
>>

Re: [PATCH v4 0/2] x86/fred: Prevent single-step upon ERETU completion

[Xin Li]

On 6/5/2025 2:51 PM, Sohil Mehta wrote:
>>> Xin Li (Intel) (2):
>>>    x86/fred/signal: Prevent single-step upon ERETU completion
>>>    selftests/x86: Add a test to detect infinite sigtrap handler loop
>>>
>> I tested the patches on a machine that supports FRED. The results are as
>> expected:
>>
> Seeing a split lock warning when running the test:
> x86/split lock detection: #DB: sigtrap_loop_64/4614 took a bus_lock trap
> at address: 0x4011ae
> 
> Wanted to get this out sooner for awareness. Will figure out more
> details and send out an update.

We investigated this issue, and figured out the reason why we see a
split lock warning when running the test:

1) The warning is not caused by the test.
2) It's a false warning.
3) It happens even when bus lock detection (BLD) is not enabled.
4) It happens only on the first #DB on a CPU.

The root cause is that Linux writes 0 to DR6 at boot time, which results
in different values in DR6 depending on whether they support BLD or not.

On CPUs that support BLD, writing 0 to DR6 sets DR6 to 0xFFFF07F0, i.e.,
bit 11 of DR6, its BLD flag, is cleared.  Otherwise 0xFFFF0FF0.

But Intel SDM says, other than BLD induced #DB, DR6.BLD is not modified.
To avoid confusion in identifying debug exceptions, software debug-
exception handlers should set bit 11 to 1 before returning.  DR6.BLD
is always 1 if the processor does not support OS bus-lock detection.

Because Linux clears DR6.BLD on CPUs that support BLD at boot time, the
first #DB will be INCORRECTLY interpreted as a BLD #DB, thus the
warning, no matter whether BLD is enabled or not.

We will be working on a fix to initialize DR6 and DR7 with their
architectural reset values instead of 0s.

Thanks!
     Xin