This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1] Thanks to Magnus Lindholm for identifying that remarkably longstanding bug.
The problem is that GCC expects 16-byte alignment of the incoming stack since early 2004, as Maciej found out [2]: Having actually dug speculatively I can see that the psABI was changed in GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double improperly aligned)") back in Mar 2004, when the stack pointer alignment was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has various suspicious stack pointer adjustments, starting with SP_OFF which is not a whole multiple of 16.
Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same: D.3.1 Stack Alignment This standard requires that stacks be octaword aligned at the time a new procedure is invoked.
However: - the "normal" kernel stack is always misaligned by 8 bytes, thanks to the odd number of 64-bit words in 'struct pt_regs', which is the very first thing pushed onto the kernel thread stack; - syscall, fault, interrupt etc. handlers may, or may not, receive aligned stack depending on numerous factors.
Somehow we got away with it until recently, when we ended up with a stack corruption in kernel/smp.c:smp_call_function_single() due to its use of 32-byte aligned local data and the compiler doing clever things allocating it on the stack.
Patche 1 is preparatory; 2 - the main fix; 3 - fixes remaining special cases.
Ivan.
[1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRc... [2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.... [3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.... --- Changes in v2: - patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf, reported by John Paul Adrian Glaubitz glaubitz@physik.fu-berlin.de; update comment and commit message accordingly; - cc'ed stable@vger.kernel.org as older kernels ought to be fixed as well.
Changes in v3: - patch #1 dropped for the time being; - updated commit messages as Maciej suggested. --- Ivan Kokshaysky (3): alpha: replace hardcoded stack offsets with autogenerated ones alpha: make stack 16-byte aligned (most cases) alpha: align stack for page fault and user unaligned trap handlers
arch/alpha/include/uapi/asm/ptrace.h | 2 ++ arch/alpha/kernel/asm-offsets.c | 4 ++++ arch/alpha/kernel/entry.S | 24 ++++++++++-------------- arch/alpha/kernel/traps.c | 2 +- arch/alpha/mm/fault.c | 4 ++-- 5 files changed, 19 insertions(+), 17 deletions(-)
This allows the assembly in entry.S to automatically keep in sync with changes in the stack layout (struct pt_regs and struct switch_stack).
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Signed-off-by: Ivan Kokshaysky ink@unseen.parts --- arch/alpha/kernel/asm-offsets.c | 4 ++++ arch/alpha/kernel/entry.S | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/alpha/kernel/asm-offsets.c b/arch/alpha/kernel/asm-offsets.c index 4cfeae42c79a..e9dad60b147f 100644 --- a/arch/alpha/kernel/asm-offsets.c +++ b/arch/alpha/kernel/asm-offsets.c @@ -19,9 +19,13 @@ static void __used foo(void) DEFINE(TI_STATUS, offsetof(struct thread_info, status)); BLANK();
+ DEFINE(SP_OFF, offsetof(struct pt_regs, ps)); DEFINE(SIZEOF_PT_REGS, sizeof(struct pt_regs)); BLANK();
+ DEFINE(SWITCH_STACK_SIZE, sizeof(struct switch_stack)); + BLANK(); + DEFINE(HAE_CACHE, offsetof(struct alpha_machine_vector, hae_cache)); DEFINE(HAE_REG, offsetof(struct alpha_machine_vector, hae_register)); } diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S index dd26062d75b3..6fb38365539d 100644 --- a/arch/alpha/kernel/entry.S +++ b/arch/alpha/kernel/entry.S @@ -15,10 +15,6 @@ .set noat .cfi_sections .debug_frame
-/* Stack offsets. */ -#define SP_OFF 184 -#define SWITCH_STACK_SIZE 64 - .macro CFI_START_OSF_FRAME func .align 4 .globl \func
On Tue, 4 Feb 2025, Ivan Kokshaysky wrote:
This allows the assembly in entry.S to automatically keep in sync with changes in the stack layout (struct pt_regs and struct switch_stack).
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Signed-off-by: Ivan Kokshaysky ink@unseen.parts
Tested-by: Maciej W. Rozycki macro@orcam.me.uk
Maciej
The problem is that GCC expects 16-byte alignment of the incoming stack since early 2004, as Maciej found out [1]: Having actually dug speculatively I can see that the psABI was changed in GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double improperly aligned)") back in Mar 2004, when the stack pointer alignment was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has various suspicious stack pointer adjustments, starting with SP_OFF which is not a whole multiple of 16.
Also, as Magnus noted, "ALPHA Calling Standard" [2] required the same: D.3.1 Stack Alignment This standard requires that stacks be octaword aligned at the time a new procedure is invoked.
However: - the "normal" kernel stack is always misaligned by 8 bytes, thanks to the odd number of 64-bit words in 'struct pt_regs', which is the very first thing pushed onto the kernel thread stack; - syscall, fault, interrupt etc. handlers may, or may not, receive aligned stack depending on numerous factors.
Somehow we got away with it until recently, when we ended up with a stack corruption in kernel/smp.c:smp_call_function_single() due to its use of 32-byte aligned local data and the compiler doing clever things allocating it on the stack.
This adds padding between the PAL-saved and kernel-saved registers so that 'struct pt_regs' have an even number of 64-bit words. This makes the stack properly aligned for most of the kernel code, except two handlers which need special threatment.
Note: struct pt_regs doesn't belong in uapi/asm; this should be fixed, but let's put this off until later.
Link: https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.... [1] Link: https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.... [2]
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Tested-by: Magnus Lindholm linmag7@gmail.com Signed-off-by: Ivan Kokshaysky ink@unseen.parts --- arch/alpha/include/uapi/asm/ptrace.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/alpha/include/uapi/asm/ptrace.h b/arch/alpha/include/uapi/asm/ptrace.h index 5ca45934fcbb..72ed913a910f 100644 --- a/arch/alpha/include/uapi/asm/ptrace.h +++ b/arch/alpha/include/uapi/asm/ptrace.h @@ -42,6 +42,8 @@ struct pt_regs { unsigned long trap_a0; unsigned long trap_a1; unsigned long trap_a2; +/* This makes the stack 16-byte aligned as GCC expects */ + unsigned long __pad0; /* These are saved by PAL-code: */ unsigned long ps; unsigned long pc;
On Tue, 4 Feb 2025, Ivan Kokshaysky wrote:
Note: struct pt_regs doesn't belong in uapi/asm; this should be fixed, but let's put this off until later.
Link: https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.... [1] Link: https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427.... [2]
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Tested-by: Magnus Lindholm linmag7@gmail.com Signed-off-by: Ivan Kokshaysky ink@unseen.parts
Tested-by: Maciej W. Rozycki macro@orcam.me.uk
Maciej
do_page_fault() and do_entUna() are special because they use non-standard stack frame layout. Fix them manually.
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Tested-by: Magnus Lindholm linmag7@gmail.com Suggested-by: Maciej W. Rozycki macro@orcam.me.uk Signed-off-by: Ivan Kokshaysky ink@unseen.parts --- arch/alpha/kernel/entry.S | 20 ++++++++++---------- arch/alpha/kernel/traps.c | 2 +- arch/alpha/mm/fault.c | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S index 6fb38365539d..f4d41b4538c2 100644 --- a/arch/alpha/kernel/entry.S +++ b/arch/alpha/kernel/entry.S @@ -194,8 +194,8 @@ CFI_END_OSF_FRAME entArith CFI_START_OSF_FRAME entMM SAVE_ALL /* save $9 - $15 so the inline exception code can manipulate them. */ - subq $sp, 56, $sp - .cfi_adjust_cfa_offset 56 + subq $sp, 64, $sp + .cfi_adjust_cfa_offset 64 stq $9, 0($sp) stq $10, 8($sp) stq $11, 16($sp) @@ -210,7 +210,7 @@ CFI_START_OSF_FRAME entMM .cfi_rel_offset $13, 32 .cfi_rel_offset $14, 40 .cfi_rel_offset $15, 48 - addq $sp, 56, $19 + addq $sp, 64, $19 /* handle the fault */ lda $8, 0x3fff bic $sp, $8, $8 @@ -223,7 +223,7 @@ CFI_START_OSF_FRAME entMM ldq $13, 32($sp) ldq $14, 40($sp) ldq $15, 48($sp) - addq $sp, 56, $sp + addq $sp, 64, $sp .cfi_restore $9 .cfi_restore $10 .cfi_restore $11 @@ -231,7 +231,7 @@ CFI_START_OSF_FRAME entMM .cfi_restore $13 .cfi_restore $14 .cfi_restore $15 - .cfi_adjust_cfa_offset -56 + .cfi_adjust_cfa_offset -64 /* finish up the syscall as normal. */ br ret_from_sys_call CFI_END_OSF_FRAME entMM @@ -378,8 +378,8 @@ entUnaUser: .cfi_restore $0 .cfi_adjust_cfa_offset -256 SAVE_ALL /* setup normal kernel stack */ - lda $sp, -56($sp) - .cfi_adjust_cfa_offset 56 + lda $sp, -64($sp) + .cfi_adjust_cfa_offset 64 stq $9, 0($sp) stq $10, 8($sp) stq $11, 16($sp) @@ -395,7 +395,7 @@ entUnaUser: .cfi_rel_offset $14, 40 .cfi_rel_offset $15, 48 lda $8, 0x3fff - addq $sp, 56, $19 + addq $sp, 64, $19 bic $sp, $8, $8 jsr $26, do_entUnaUser ldq $9, 0($sp) @@ -405,7 +405,7 @@ entUnaUser: ldq $13, 32($sp) ldq $14, 40($sp) ldq $15, 48($sp) - lda $sp, 56($sp) + lda $sp, 64($sp) .cfi_restore $9 .cfi_restore $10 .cfi_restore $11 @@ -413,7 +413,7 @@ entUnaUser: .cfi_restore $13 .cfi_restore $14 .cfi_restore $15 - .cfi_adjust_cfa_offset -56 + .cfi_adjust_cfa_offset -64 br ret_from_sys_call CFI_END_OSF_FRAME entUna
diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c index a9a38c80c4a7..7004397937cf 100644 --- a/arch/alpha/kernel/traps.c +++ b/arch/alpha/kernel/traps.c @@ -649,7 +649,7 @@ s_reg_to_mem (unsigned long s_reg) static int unauser_reg_offsets[32] = { R(r0), R(r1), R(r2), R(r3), R(r4), R(r5), R(r6), R(r7), R(r8), /* r9 ... r15 are stored in front of regs. */ - -56, -48, -40, -32, -24, -16, -8, + -64, -56, -48, -40, -32, -24, -16, /* padding at -8 */ R(r16), R(r17), R(r18), R(r19), R(r20), R(r21), R(r22), R(r23), R(r24), R(r25), R(r26), R(r27), R(r28), R(gp), diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index 8c9850437e67..a9816bbc9f34 100644 --- a/arch/alpha/mm/fault.c +++ b/arch/alpha/mm/fault.c @@ -78,8 +78,8 @@ __load_new_mm_context(struct mm_struct *next_mm)
/* Macro for exception fixup code to access integer registers. */ #define dpf_reg(r) \ - (((unsigned long *)regs)[(r) <= 8 ? (r) : (r) <= 15 ? (r)-16 : \ - (r) <= 18 ? (r)+10 : (r)-10]) + (((unsigned long *)regs)[(r) <= 8 ? (r) : (r) <= 15 ? (r)-17 : \ + (r) <= 18 ? (r)+11 : (r)-10])
asmlinkage void do_page_fault(unsigned long address, unsigned long mmcsr,
On Tue, 4 Feb 2025, Ivan Kokshaysky wrote:
do_page_fault() and do_entUna() are special because they use non-standard stack frame layout. Fix them manually.
Cc: stable@vger.kernel.org Reviewed-by: Maciej W. Rozycki macro@orcam.me.uk Tested-by: Magnus Lindholm linmag7@gmail.com Suggested-by: Maciej W. Rozycki macro@orcam.me.uk Signed-off-by: Ivan Kokshaysky ink@unseen.parts
Tested-by: Maciej W. Rozycki macro@orcam.me.uk
Maciej
On Tue, 4 Feb 2025, Ivan Kokshaysky wrote:
This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1] Thanks to Magnus Lindholm for identifying that remarkably longstanding bug.
Thank you. TL;DR: I've completed regression testing now and the results are looking good, so I have posted my Tested-by: tags now.
Sadly original verification triggered a glitch which sometimes happens in the operation of my server's intranet network card that puts the device offline until it's hot-removed and the re-discovered. I suspect a driver bug triggering under higher traffic, but that yet has to be investigated and it's a bit of a challenge with a machine that is at a remote location and in constant use.
I have only discovered it once 1023 tests got affected and consequently failed due to the inability to reach the target Alpha system. I concluded it was infeasible to run the failed tests by hand and then try to merge the results.
Instead I disabled the extra prolonged tests discussed in the previous update and rerun the remaining whole testsuite. That completed in ~23h and produced a few regressions and progressions, the former for the "math" and "nptl" test subsets, as well as one "string" test ("test-strnlen"). I have therefore verified them by hand and triggered the failures with your patchset removed, so I concluded their failed results are not the outcome of your changes.
So this is good to go in as far as I'm concerned, as noted at the top. Thank you for taking time to work on this problem.
Maciej
Hi,
On Tue, 2025-02-04 at 23:35 +0100, Ivan Kokshaysky wrote:
This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1] Thanks to Magnus Lindholm for identifying that remarkably longstanding bug.
The problem is that GCC expects 16-byte alignment of the incoming stack since early 2004, as Maciej found out [2]: Having actually dug speculatively I can see that the psABI was changed in GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double improperly aligned)") back in Mar 2004, when the stack pointer alignment was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has various suspicious stack pointer adjustments, starting with SP_OFF which is not a whole multiple of 16.
Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same: D.3.1 Stack Alignment This standard requires that stacks be octaword aligned at the time a new procedure is invoked.
However:
- the "normal" kernel stack is always misaligned by 8 bytes, thanks to the odd number of 64-bit words in 'struct pt_regs', which is the very first thing pushed onto the kernel thread stack;
- syscall, fault, interrupt etc. handlers may, or may not, receive aligned stack depending on numerous factors.
Somehow we got away with it until recently, when we ended up with a stack corruption in kernel/smp.c:smp_call_function_single() due to its use of 32-byte aligned local data and the compiler doing clever things allocating it on the stack.
Patche 1 is preparatory; 2 - the main fix; 3 - fixes remaining special cases.
Ivan.
[1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRc... [2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.... [3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427....
Changes in v2:
- patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf, reported by John Paul Adrian Glaubitz glaubitz@physik.fu-berlin.de; update comment and commit message accordingly;
- cc'ed stable@vger.kernel.org as older kernels ought to be fixed as well.
Changes in v3:
- patch #1 dropped for the time being;
- updated commit messages as Maciej suggested.
Ivan Kokshaysky (3): alpha: replace hardcoded stack offsets with autogenerated ones alpha: make stack 16-byte aligned (most cases) alpha: align stack for page fault and user unaligned trap handlers
arch/alpha/include/uapi/asm/ptrace.h | 2 ++ arch/alpha/kernel/asm-offsets.c | 4 ++++ arch/alpha/kernel/entry.S | 24 ++++++++++-------------- arch/alpha/kernel/traps.c | 2 +- arch/alpha/mm/fault.c | 4 ++-- 5 files changed, 19 insertions(+), 17 deletions(-)
Can we get this landed this week, maybe for v6.14-rc3? This way it will quickly backported to various stable kernels which means it will reach Debian unstable within a few days.
Thanks, Adrian
On Tue, Feb 11, 2025 at 2:20 AM John Paul Adrian Glaubitz glaubitz@physik.fu-berlin.de wrote:
Hi,
On Tue, 2025-02-04 at 23:35 +0100, Ivan Kokshaysky wrote:
This series fixes oopses on Alpha/SMP observed since kernel v6.9. [1] Thanks to Magnus Lindholm for identifying that remarkably longstanding bug.
The problem is that GCC expects 16-byte alignment of the incoming stack since early 2004, as Maciej found out [2]: Having actually dug speculatively I can see that the psABI was changed in GCC 3.5 with commit e5e10fb4a350 ("re PR target/14539 (128-bit long double improperly aligned)") back in Mar 2004, when the stack pointer alignment was increased from 8 bytes to 16 bytes, and arch/alpha/kernel/entry.S has various suspicious stack pointer adjustments, starting with SP_OFF which is not a whole multiple of 16.
Also, as Magnus noted, "ALPHA Calling Standard" [3] required the same: D.3.1 Stack Alignment This standard requires that stacks be octaword aligned at the time a new procedure is invoked.
However:
- the "normal" kernel stack is always misaligned by 8 bytes, thanks to the odd number of 64-bit words in 'struct pt_regs', which is the very first thing pushed onto the kernel thread stack;
- syscall, fault, interrupt etc. handlers may, or may not, receive aligned stack depending on numerous factors.
Somehow we got away with it until recently, when we ended up with a stack corruption in kernel/smp.c:smp_call_function_single() due to its use of 32-byte aligned local data and the compiler doing clever things allocating it on the stack.
Patche 1 is preparatory; 2 - the main fix; 3 - fixes remaining special cases.
Ivan.
[1] https://lore.kernel.org/rcu/CA+=Fv5R9NG+1SHU9QV9hjmavycHKpnNyerQ=Ei90G98ukRc... [2] https://lore.kernel.org/rcu/alpine.DEB.2.21.2501130248010.18889@angie.orcam.... [3] https://bitsavers.org/pdf/dec/alpha/Alpha_Calling_Standard_Rev_2.0_19900427....
Changes in v2:
- patch #1: provide empty 'struct pt_regs' to fix compile failure in libbpf, reported by John Paul Adrian Glaubitz glaubitz@physik.fu-berlin.de; update comment and commit message accordingly;
- cc'ed stable@vger.kernel.org as older kernels ought to be fixed as well.
Changes in v3:
- patch #1 dropped for the time being;
- updated commit messages as Maciej suggested.
Ivan Kokshaysky (3): alpha: replace hardcoded stack offsets with autogenerated ones alpha: make stack 16-byte aligned (most cases) alpha: align stack for page fault and user unaligned trap handlers
arch/alpha/include/uapi/asm/ptrace.h | 2 ++ arch/alpha/kernel/asm-offsets.c | 4 ++++ arch/alpha/kernel/entry.S | 24 ++++++++++-------------- arch/alpha/kernel/traps.c | 2 +- arch/alpha/mm/fault.c | 4 ++-- 5 files changed, 19 insertions(+), 17 deletions(-)
Can we get this landed this week, maybe for v6.14-rc3? This way it will quickly backported to various stable kernels which means it will reach Debian unstable within a few days.
Thanks, Adrian
Yeah, I'll vacuum the patches up and send them to Linus.
I've been running them on my ES47 and while my ES47 is still unstable, these patches definitely solve this bug [1] and fix failures I saw with the rcu_torture module!
Thanks a bunch to all of you!
linux-stable-mirror@lists.linaro.org
-
Ivan Kokshaysky -
John Paul Adrian Glaubitz -
Maciej W. Rozycki -
Matt Turner