Re: Kernel Oops on alpha with kernel version >=6.9.x

[Ivan Kokshaysky <ink@unseen.parts>]

On Sat, Jan 25, 2025 at 03:35:08PM +0000, Maciej W. Rozycki wrote:
> On Fri, 24 Jan 2025, Ivan Kokshaysky wrote:
> 
> > > > > Indeed, SP_OFF in entry.S is the main suspect at the moment.
> > > > 
> > > > In fact, it's the odd number of longs (29) in struct pt_regs that makes
> > > > the stack misaligned by 8 bytes. The patch below works for me - no more
> > > > oopses in rcu-torture test.
> > > > 
> > > > Unless I'm missing something, this change shouldn't have any ill effects.
> > > 
> > >  Umm, this is a part of UAPI, and the change in alignment changes the ABI 
> > > (think padding where `struct pt_regs' has been embedded into another 
> > > structure), so AFAICT it is a no-no.
> > 
> > Well, the only userspace applications I can think of that need kernel
> > stack layout are debuggers, but at least alpha gdb doesn't use this header.
> > Doesn't matter, though - padding *after* PAL-saved registers is wrong
> > thing to do. I think it's the reason for oopses that Magnus reported
> > today.
> > 
> > A "long" padding memder of pt_regs placed *before* PAL-saved registers
> > would be a proper fix for kernel, but it most likely would break gdb...
> > 
> > >  But the only place I could quickly find this should matter for is this:
> > > 
> > > 	/* ... and find our stack ... */
> > > 	lda	$30,0x4000 - SIZEOF_PT_REGS($8)
> > > 
> > > which should be straightforward to fix:
> > > 
> > > 	lda	$30,0x4000 - ((SIZEOF_PT_REGS + 15) & ~15)($8)
> > > 
> > > or suchlike.  Have I missed anything?
> > 
> > That's the first thing I thought of too, but no, it's just a kernel
> > entry point after the bootloader. The stack pointer of kernel threads
> > is assigned in alpha/kernel/process.c. Particularly, these macros
> > in ptrace.h (non-uapi) are interesting:
> > 
> > #define task_pt_regs(task) \
> >   ((struct pt_regs *) (task_stack_page(task) + 2*PAGE_SIZE) - 1)
> > 
> > #define current_pt_regs() \
> >   ((struct pt_regs *) ((char *)current_thread_info() + 2*PAGE_SIZE) - 1)
> > 
> > I'll try to play with alignment here, but it will take some time.
> 
>  So after a crash course in PALcode stack frames I have come up with the 
> following WIP patch that works for me.  If things go well, I'll clean it 
> up a little and turn into a proper patch submission.  Not that I think I 
> can make the end result particularly pretty, there's no easy way AFAICT.
> 
>  NB with some instrumentation here's what gets reported for stack without:
> 
> start_kernel: SP: fffffc0000dcfe98
> do_entInt: SP: fffffc0000dcfc30
> copy_thread: SP: fffffc0000dcfc98, regs: fffffc0000dcff18, childregs: fffffc0001837f18, childstack: fffffc0001837ed8
> do_page_fault: SP: fffffc0001837bc8
> sys_exit_group: SP: fffffc0002917ef8
> do_entUnaUser: SP: fffffc0001f33e70
> do_entArith: SP: fffffc0001f33ee8
> do_entIF: SP: fffffc000184bee8
> 
> and with the patch:
> 
> start_kernel: SP: fffffc0000dcfe90
> do_entInt: SP: fffffc0000dcfc20
> copy_thread: SP: fffffc0000dcfc90, regs: fffffc0000dcff18, childregs: fffffc000183bf18, childstack: fffffc000183bed0
> do_page_fault: SP: fffffc000183bbc0
> sys_exit_group: SP: fffffc00028d3ef0
> do_entUnaUser: SP: fffffc000292fe70
> do_entArith: SP: fffffc0001d7fee0
> do_entIF: SP: fffffc0002827ee0
> 
> for the relevant situations (except for `entDbg', but that's analogous and 
> largely unused anyway).
> 
>  Can you guys please give it a try?

Oh. I have little doubt it works, but so much hard work just to keep
the pt_regs thing intact? Instead of asking ourselves how come it ended
up in uapi?

It was commit 96433f6ee49032d7a8b back in 2012 done by some scripting,
I believe it was by mistake, because it's the kernel bowels exposed for
absolutely no reason. I was going to propose a patch below (I don't think
we can remove the file, as it probably break build of packages like
linux-libc), and then add padding to pt_regs with exactly the same effect
as your patch.

Ivan.

diff --git a/arch/alpha/include/uapi/asm/ptrace.h b/arch/alpha/include/uapi/asm/ptrace.h
index 5ca45934fcbb..6b09e1df343d 100644
--- a/arch/alpha/include/uapi/asm/ptrace.h
+++ b/arch/alpha/include/uapi/asm/ptrace.h
@@ -2,7 +2,7 @@
 #ifndef _UAPI_ASMAXP_PTRACE_H
 #define _UAPI_ASMAXP_PTRACE_H
 
-
+#ifdef __KERNEL__
 /*
  * This struct defines the way the registers are stored on the
  * kernel stack during a system call or other kernel entry
@@ -64,10 +64,7 @@ struct switch_stack {
 	unsigned long r14;
 	unsigned long r15;
 	unsigned long r26;
-#ifndef __KERNEL__
-	unsigned long fp[32];	/* fp[31] is fpcr */
-#endif
 };
-
+#endif
 
 #endif /* _UAPI_ASMAXP_PTRACE_H */