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