From: Yang Li <[email protected]> [ Upstream commit 809ef03d6d21d5fea016bbf6babeec462e37e68c ]
In the csky_cmpxchg_fixup function, it is incorrect to use the global variable csky_cmpxchg_stw to determine the address where the exception occurred.The global variable csky_cmpxchg_stw stores the opcode at the time of the exception, while &csky_cmpxchg_stw shows the address where the exception occurred. Signed-off-by: Yang Li <[email protected]> Signed-off-by: Guo Ren <[email protected]> Signed-off-by: Sasha Levin <[email protected]> --- LLM Generated explanations, may be completely bogus: ## Analysis of csky: fix csky_cmpxchg_fixup not working ### 1. COMMIT MESSAGE ANALYSIS **Subject**: "csky: fix csky_cmpxchg_fixup not working" - clearly indicates a bug fix **Key issue explained**: The commit message describes a fundamental semantic error - the code was using the **value** of `csky_cmpxchg_stw` (which contains an opcode) when it should have been using the **address** of `csky_cmpxchg_stw` (where the instruction is located). **Missing tags**: No "Cc: [email protected]" or "Fixes:" tag, but this doesn't preclude backporting if the fix is clearly warranted. ### 2. CODE CHANGE ANALYSIS The change is extremely surgical - only 2 lines modified: ```c - if (instruction_pointer(regs) == csky_cmpxchg_stw) - instruction_pointer_set(regs, csky_cmpxchg_ldw); + if (instruction_pointer(regs) == (unsigned long)&csky_cmpxchg_stw) + instruction_pointer_set(regs, (unsigned long)&csky_cmpxchg_ldw); ``` **Technical explanation**: - `csky_cmpxchg_ldw` and `csky_cmpxchg_stw` are external symbols declared as `extern unsigned long` - they represent labels/addresses in the cmpxchg assembly implementation - The **value** stored at these symbols is the opcode of the instruction - The **address** (`&csky_cmpxchg_stw`) is where the instruction resides in memory - The code compares against `instruction_pointer(regs)` which is an address, so it must compare against an address, not an opcode value **Root cause**: Simple semantic error - using value instead of address **Why the bug is severe**: This function handles TLB modification faults during compare-and-exchange operations. When such a fault occurs at the store instruction, the handler should redirect execution back to the load instruction to retry the operation. With the bug, the comparison `instruction_pointer(regs) == csky_cmpxchg_stw` would almost never match (comparing an address to an opcode), so the fixup **never worked**. ### 3. CLASSIFICATION - **Bug fix**: Yes, clearly fixing broken functionality - **Security impact**: Potentially - broken cmpxchg can lead to race conditions - **Data corruption risk**: Yes - atomic operations that don't work correctly can cause data races ### 4. SCOPE AND RISK ASSESSMENT - **Lines changed**: 2 lines - **Files touched**: 1 file (arch/csky/mm/fault.c) - **Subsystem**: CSKY architecture-specific code - **Complexity**: Minimal - straightforward address-of fix - **Risk**: Very low - the fix is obviously correct and architecture- specific ### 5. USER IMPACT - **Affected users**: CSKY systems without LDSTEX instructions (when `CONFIG_CPU_HAS_LDSTEX` is not defined) - **Severity**: High - broken compare-and-exchange atomic operations can cause: - Race conditions in concurrent code - Data corruption - Deadlocks - Unpredictable behavior in any code using cmpxchg ### 6. STABILITY INDICATORS - Signed-off by maintainer Guo Ren (CSKY maintainer) - The fix is logically obvious once understood ### 7. DEPENDENCY CHECK - No dependencies on other commits - The affected code has existed since CSKY was added to the kernel - Should apply cleanly to stable trees that have CSKY support ### CONCLUSION This commit clearly meets all stable kernel criteria: 1. **Obviously correct**: The fix is a textbook case of using `&variable` (address) instead of `variable` (value) when comparing against an instruction pointer 2. **Fixes a real bug**: The cmpxchg fixup mechanism was completely non- functional 3. **Important issue**: Broken atomic operations can cause data corruption, race conditions, and system instability 4. **Small and contained**: Only 2 lines changed in one file, architecture-specific 5. **No new features**: Just fixing existing functionality to actually work The absence of stable tags appears to be an oversight. This is a critical fix for CSKY platforms - without it, the entire cmpxchg fixup path is dead code that never triggers when it should. **YES** arch/csky/mm/fault.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/csky/mm/fault.c b/arch/csky/mm/fault.c index a6ca7dff42153..7ff4011089850 100644 --- a/arch/csky/mm/fault.c +++ b/arch/csky/mm/fault.c @@ -45,8 +45,8 @@ static inline void csky_cmpxchg_fixup(struct pt_regs *regs) if (trap_no(regs) != VEC_TLBMODIFIED) return; - if (instruction_pointer(regs) == csky_cmpxchg_stw) - instruction_pointer_set(regs, csky_cmpxchg_ldw); + if (instruction_pointer(regs) == (unsigned long)&csky_cmpxchg_stw) + instruction_pointer_set(regs, (unsigned long)&csky_cmpxchg_ldw); return; } #endif -- 2.51.0
