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