From: Huacai Chen chenhuacai@loongson.cn
[ Upstream commit 112ca94f6c3b3e0b2002a240de43c487a33e0234 ]
Now if preemption happens between protected_save_fpu_context() and protected_save_lbt_context(), FTOP context is lost. Because FTOP is saved by protected_save_lbt_context() but protected_save_fpu_context() disables TM before that. So save LBT before FPU in setup_sigcontext() to avoid this potential risk.
Signed-off-by: Hanlu Li lihanlu@loongson.cn Signed-off-by: Huacai Chen chenhuacai@loongson.cn Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Based on my analysis of this commit and the Linux kernel codebase, I can now provide a comprehensive assessment.
**Backport Status: YES**
## Extensive Analysis
This commit fixes a **critical race condition** in the LoongArch signal handling code that can lead to **data loss** of processor state during signal context saving. Here's the detailed analysis:
### The Bug Being Fixed
1. **Context Loss Issue**: The commit addresses a race condition where the FTOP (x87 FPU stack pointer) context can be lost during signal handling. This happens when: - A preemption occurs between `protected_save_fpu_context()` and `protected_save_lbt_context()` - The FPU context save operation disables TM (likely Transaction Memory or a similar mechanism) before LBT context is saved - Since FTOP is part of the LBT (Loongson Binary Translation) extension context, it gets lost
2. **Binary Translation Context**: LBT is a hardware extension used to accelerate binary translation on LoongArch processors. According to the original LBT support commit (bd3c5798484a), it includes: - 4 scratch registers (scr0-scr3) - x86/ARM eflags register - x87 FPU stack pointer (FTOP)
### Code Changes Analysis
The fix is **minimal and surgical** - it simply reorders the save operations:
**Before (buggy order):** ```c // Save FPU contexts first (LASX/LSX/FPU) if (extctx->lasx.addr) err |= protected_save_lasx_context(extctx); else if (extctx->lsx.addr) err |= protected_save_lsx_context(extctx); else if (extctx->fpu.addr) err |= protected_save_fpu_context(extctx);
// Save LBT context last - PROBLEM: FTOP may be lost by now #ifdef CONFIG_CPU_HAS_LBT if (extctx->lbt.addr) err |= protected_save_lbt_context(extctx); #endif ```
**After (fixed order):** ```c // Save LBT context FIRST to preserve FTOP #ifdef CONFIG_CPU_HAS_LBT if (extctx->lbt.addr) err |= protected_save_lbt_context(extctx); #endif
// Then save FPU contexts (LASX/LSX/FPU) if (extctx->lasx.addr) err |= protected_save_lasx_context(extctx); else if (extctx->lsx.addr) err |= protected_save_lsx_context(extctx); else if (extctx->fpu.addr) err |= protected_save_fpu_context(extctx); ```
### Why This Should Be Backported
1. **Data Corruption Risk**: This bug can cause loss of processor state during signal handling, which could lead to: - Incorrect program execution after signal return - Potential application crashes - Data corruption in applications using binary translation features
2. **Small, Contained Fix**: The change is: - Only 10 lines (5 insertions, 5 deletions) - Confined to a single function in signal handling - Simply reorders existing operations without adding new logic - Protected by `#ifdef CONFIG_CPU_HAS_LBT` so it only affects systems with LBT support
3. **No Architectural Changes**: This is purely a bug fix that: - Doesn't introduce new features - Doesn't change kernel APIs or ABIs - Doesn't modify core subsystem behavior - Only affects LoongArch architecture with LBT extension enabled
4. **Clear Bug with Clear Fix**: The problem is well-defined (race condition causing context loss) and the solution is straightforward (reorder operations to save LBT before FPU).
5. **Affects User-Space Reliability**: Signal handling is a fundamental mechanism used by many applications. A bug here can affect system stability and application reliability.
### Risk Assessment
The risk of regression is **very low** because: - The change only affects code paths when LBT is enabled (`CONFIG_CPU_HAS_LBT`) - It's a simple reordering of independent save operations - The fix has been tested and merged into mainline - It doesn't change the fundamental logic, just the execution order
This commit clearly meets the stable kernel criteria for backporting as it fixes an important bug with minimal risk of introducing new issues.
arch/loongarch/kernel/signal.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/kernel/signal.c b/arch/loongarch/kernel/signal.c index 4740cb5b2388..c9f7ca778364 100644 --- a/arch/loongarch/kernel/signal.c +++ b/arch/loongarch/kernel/signal.c @@ -677,6 +677,11 @@ static int setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc, for (i = 1; i < 32; i++) err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
+#ifdef CONFIG_CPU_HAS_LBT + if (extctx->lbt.addr) + err |= protected_save_lbt_context(extctx); +#endif + if (extctx->lasx.addr) err |= protected_save_lasx_context(extctx); else if (extctx->lsx.addr) @@ -684,11 +689,6 @@ static int setup_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc, else if (extctx->fpu.addr) err |= protected_save_fpu_context(extctx);
-#ifdef CONFIG_CPU_HAS_LBT - if (extctx->lbt.addr) - err |= protected_save_lbt_context(extctx); -#endif - /* Set the "end" magic */ info = (struct sctx_info *)extctx->end.addr; err |= __put_user(0, &info->magic);