On Sat, Jan 25, 2025 at 06:59:32PM +0000, Maciej W. Rozycki wrote:
> On Sat, 25 Jan 2025, Maciej W. Rozycki wrote:
>
> > > Interesting. Perhaps these frames are aligned by PAL-code as well,
> > > the reference manual wasn't clear about that.
> >
> > I think it just boils down to the amount of exception nesting.
>
> Ah, actually most of these faults were entered from the user mode and the
> kernel stack starts at a page boundary, so once the stack frame has been
> allocated by PALcode and SAVE_ALL combined the stack pointer ends up
> misaligned. For faults entered from the kernel mode the opposite might be
> the case.
Indeed, sounds plausible.
> So unless we want to play (and we don't) with FP and the saving
> and restoration of SP, we just want to keep SP aligned at all times.
> > > I knew about entUna, I thought it's safe as it only deals with 64-bit data
> > > and not going to be changed in future, but missed entMM...
> > >
> > > I agree, better fix both.
> >
> > Well, we may get away with it in many cases, which is obviously why
> > this bug has survived so long, but in principle it is not safe to enter
> > C code with the stack misaligned, so yes, we need to fix all the code
> > paths, also because a nested exception will cause hell to break loose.
> >
> > Here just bumping up the frame size and adjusting offsets in assembly
> > code accordingly so as to account for the empty longword at the bottom
> > of the frame should do, just as I did across my change.
>
> ... or, depending on how you look at it, top of the frame and FAOD in any
> case the longword closest to the stack pointer will be the empty one.
Right. So if we agree on my variant, this addition patch is needed.
[No problems with gdb, as expected.]
Ivan.
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
