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

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <B6762FE6-87DD-4B2D-B177-A574044C1513@zytor.com>
Date: Thu, 05 Jun 2025 11:46:38 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: "Xin Li (Intel)" <xin@...or.com>, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org
CC: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, shuah@...nel.org,
        andrew.cooper3@...rix.com, sohil.mehta@...el.com
Subject: Re: [PATCH v4 1/2] x86/fred/signal: Prevent single-step upon ERETU completion

On June 5, 2025 11:10:19 AM PDT, "Xin Li (Intel)" <xin@...or.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@...or.com>
>Signed-off-by: Xin Li (Intel) <xin@...or.com>
>Cc: stable@...r.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"