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 */
