From: Xin Li xin@zytor.com
Clear the software event flag in the augmented SS to prevent infinite SIGTRAP handler loop if TF is used without an external debugger.
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 --- arch/x86/include/asm/sighandling.h | 20 ++++++++++++++++++++ arch/x86/kernel/signal_32.c | 4 ++++ arch/x86/kernel/signal_64.c | 4 ++++ 3 files changed, 28 insertions(+)
diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h index e770c4fc47f4..ecb0411fe88c 100644 --- a/arch/x86/include/asm/sighandling.h +++ b/arch/x86/include/asm/sighandling.h @@ -24,4 +24,24 @@ 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 TF is used without an external + * debugger, 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. + */ + if (IS_ENABLED(CONFIG_X86_FRED) && cpu_feature_enabled(X86_FEATURE_FRED) && + !(regs->flags & X86_EFLAGS_TF)) + regs->fred_ss.swevent = 0; +} + #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)))
base-commit: 6a7c3c2606105a41dde81002c0037420bc1ddf00
On 5/21/25 23:05, Xin Li (Intel) wrote:
+/*
- To prevent infinite SIGTRAP handler loop if TF is used without an external
- debugger, 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.*/- if (IS_ENABLED(CONFIG_X86_FRED) && cpu_feature_enabled(X86_FEATURE_FRED) &&
!(regs->flags & X86_EFLAGS_TF))regs->fred_ss.swevent = 0;+}
Minor nit (and I should have caught this when I saw your patch earlier):
cpu_feature_enabled(X86_FEATURE_FRED) is unnecessary here, because when FRED is not enabled, regs->fred_ss.swevent will always be 0, and this bit has no function, so there is no point in making that extra test.
The only reason for IS_ENABLED(CONFIG_X86_FRED) (which is implied in cpu_feature_enabled() anyway via !CONFIG_X86_DISABLED_FEATURE_FRED) is to eliminate the code entirely if FRED is not compiled in.
Although the way it goes, it sounds like CONFIG_X86_FRED might go away soon anyway, since KVM wants to use some of the FRED code unconditionally (and Xen might follow, too.)
-hpa
On 5/22/2025 12:20 AM, H. Peter Anvin wrote:
On 5/21/25 23:05, Xin Li (Intel) wrote:
+/*
- To prevent infinite SIGTRAP handler loop if TF is used without an external
- debugger, 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.*/- if (IS_ENABLED(CONFIG_X86_FRED) && cpu_feature_enabled(X86_FEATURE_FRED) &&
!(regs->flags & X86_EFLAGS_TF))regs->fred_ss.swevent = 0;+}
Minor nit (and I should have caught this when I saw your patch earlier):
cpu_feature_enabled(X86_FEATURE_FRED) is unnecessary here, because when FRED is not enabled, regs->fred_ss.swevent will always be 0, and this bit has no function, so there is no point in making that extra test.
Yeah, less conditions, less complexity. I will remove it.
Thanks! Xin
Hi Xin,
kernel test robot noticed the following build errors:
[auto build test ERROR on 6a7c3c2606105a41dde81002c0037420bc1ddf00]
url: https://github.com/intel-lab-lkp/linux/commits/Xin-Li-Intel/x86-fred-signal-... base: 6a7c3c2606105a41dde81002c0037420bc1ddf00 patch link: https://lore.kernel.org/r/20250522060549.2882444-1-xin%40zytor.com patch subject: [PATCH v1 1/1] x86/fred/signal: Prevent single-step upon ERETU completion config: i386-buildonly-randconfig-003-20250522 (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202505230141.4YBHhrPI-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/signal_32.c:32: arch/x86/include/asm/sighandling.h: In function 'prevent_single_step_upon_eretu':
arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
vim +44 arch/x86/include/asm/sighandling.h
26 27 /* 28 * To prevent infinite SIGTRAP handler loop if TF is used without an external 29 * debugger, clear the software event flag in the augmented SS, ensuring no 30 * single-step trap is pending upon ERETU completion. 31 * 32 * Note, this function should be called in sigreturn() before the original state 33 * is restored to make sure the TF is read from the entry frame. 34 */ 35 static __always_inline void prevent_single_step_upon_eretu(struct pt_regs *regs) 36 { 37 /* 38 * If the trap flag (TF) is set, i.e., the sigreturn() SYSCALL instruction 39 * is being single-stepped, do not clear the software event flag in the 40 * augmented SS, thus a debugger won't skip over the following instruction. 41 */ 42 if (IS_ENABLED(CONFIG_X86_FRED) && cpu_feature_enabled(X86_FEATURE_FRED) && 43 !(regs->flags & X86_EFLAGS_TF))
44 regs->fred_ss.swevent = 0;
45 } 46
On 5/22/2025 10:20 AM, kernel test robot wrote:
Hi Xin,
kernel test robot noticed the following build errors:
[auto build test ERROR on 6a7c3c2606105a41dde81002c0037420bc1ddf00]
url: https://github.com/intel-lab-lkp/linux/commits/Xin-Li-Intel/x86-fred-signal-... base: 6a7c3c2606105a41dde81002c0037420bc1ddf00 patch link: https://lore.kernel.org/r/20250522060549.2882444-1-xin%40zytor.com patch subject: [PATCH v1 1/1] x86/fred/signal: Prevent single-step upon ERETU completion config: i386-buildonly-randconfig-003-20250522 (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202505230141.4YBHhrPI-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/signal_32.c:32: arch/x86/include/asm/sighandling.h: In function 'prevent_single_step_upon_eretu':arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
Hmm, this statement is under IS_ENABLED(CONFIG_X86_FRED), which should be a compile time FALSE with i386. Why it is still being compiled?
Thanks! Xin
On 22/05/2025 6:33 pm, Xin Li wrote:
On 5/22/2025 10:20 AM, kernel test robot wrote:
Hi Xin,
kernel test robot noticed the following build errors:
[auto build test ERROR on 6a7c3c2606105a41dde81002c0037420bc1ddf00]
url: https://github.com/intel-lab-lkp/linux/commits/Xin-Li-Intel/x86-fred-signal-... base: 6a7c3c2606105a41dde81002c0037420bc1ddf00 patch link: https://lore.kernel.org/r/20250522060549.2882444-1-xin%40zytor.com patch subject: [PATCH v1 1/1] x86/fred/signal: Prevent single-step upon ERETU completion config: i386-buildonly-randconfig-003-20250522 (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505230141.4YBHhrPI-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202505230141.4YBHhrPI-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/signal_32.c:32: arch/x86/include/asm/sighandling.h: In function 'prevent_single_step_upon_eretu':
arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
Hmm, this statement is under IS_ENABLED(CONFIG_X86_FRED), which should be a compile time FALSE with i386. Why it is still being compiled?
Because what the compiler is seeing is:
if (0 && ...) regs->fred_ss.swevent = 0;
and the bad field name is found at parse time, while the whole expression is only discarded during optimisation.
The one thing you can't IS_ENABLED() around is conditional fields. That needs to be full #ifdef.
~Andrew
arch/x86/include/asm/sighandling.h: In function 'prevent_single_step_upon_eretu':
arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
Hmm, this statement is under IS_ENABLED(CONFIG_X86_FRED), which should be a compile time FALSE with i386. Why it is still being compiled?
Because what the compiler is seeing is:
if (0 && ...) regs->fred_ss.swevent = 0;
and the bad field name is found at parse time, while the whole expression is only discarded during optimisation.
The one thing you can't IS_ENABLED() around is conditional fields. That needs to be full #ifdef.
Thanks a lot for the explanation. Just sent out v3 using #ifdef.
Xin
On 5/22/25 10:33, Xin Li wrote:
arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
Hmm, this statement is under IS_ENABLED(CONFIG_X86_FRED), which should
The compiler still _compiles_ unreachable statements:
if (0) foo = bar.bar;
An #ifdef will keep the compiler out completely, of course. That's probably the best thing in this case, especially since it'll be tucked away in a header.
On 5/22/2025 10:54 AM, Dave Hansen wrote:
On 5/22/25 10:33, Xin Li wrote:
arch/x86/include/asm/sighandling.h:44:21: error: 'struct pt_regs' has no member named 'fred_ss'
44 | regs->fred_ss.swevent = 0; | ^~
Hmm, this statement is under IS_ENABLED(CONFIG_X86_FRED), which should
The compiler still _compiles_ unreachable statements:
if (0) foo = bar.bar;
An #ifdef will keep the compiler out completely, of course. That's probably the best thing in this case, especially since it'll be tucked away in a header.
Thanks a lot for the explanation.
Xin
linux-stable-mirror@lists.linaro.org
-
Andrew Cooper -
Dave Hansen -
H. Peter Anvin -
kernel test robot -
Xin Li -
Xin Li (Intel)