Inserting a gap of 20 bytes avoids user stack corruption due to a discrepancy between user stack pointer and stack state when an instruction is interrupted by a bus fault in mid-execution, in particular a movem instruction with the user stack pointer as target, such as commonly used in the function prologue on C function calls. The user stack pointer in this case reflects the stack state before the instruction, but as many write operations as would fit while on a mapped page are already written out at that point. On 030 processors, the resulting exception (format b) causes the instruction to be resumed, not restarted after return from exception, and only at that point is the user stack pointer updated. We must not corrupt the user stack by writing beyond what's already stored to the stack, and using the pre-execution stack pointer to place the signal stack, that is exactly what happens. We have two ways to address the issue: avoid running signal delivery on format b bus fault exception (as suggested by Andreas), or insert a suitable gap into the signal frame to avoid it clobbering the stack contets. Inserting a gap into every signal frame indiscriminately does appear to cause memory leaks or excessive memory use on low memory systems (been there ...). Some heuristics is called for to only use the frame gap trick where appropriate. We can use the format b fault address in frame.un.fmtb.daddr to find the address where the fault occurred, i.e. the address where the instruction will resume. Addresses above are no-go, anything up to and including the fault address is OK to use. But we better make sure that the fault address is actually on the page below the USP, and that the start of the signal frame will end up on the same page that the fault occurred on (again, been there). The latter might be sufficient to ensure we're not hitting an unmapped page when copying the signal frame, as the bus fault will have been resolved when we get to signal delivery (??). The bug that started this debug effort was reported by Stan Johnson. Finn Thain developed a test case and debugged the stack corruption. Fix developed and tested on my Falcon 030 with just 14 MB of RAM. Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx> CC: Finn Thain <fthain@xxxxxxxxxxxxxx> CC: Andreas Schwab <schwab@xxxxxxxxxxxxxx> CC: Stan Johnson <userm57@xxxxxxxxx> Link: https://lore.kernel.org/r/CAMuHMdW3yD22_ApemzW_6me3adq6A458u1_F0v-1EYwK_62jPA@xxxxxxxxxxxxxx Link: https://lore.kernel.org/r/e10b8e06-6a36-5c83-89da-bec8fd7d3ed9@xxxxxxxxxxxxxx --- arch/m68k/kernel/signal.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c index dfc7590abce8..75f4c4943231 100644 --- a/arch/m68k/kernel/signal.c +++ b/arch/m68k/kernel/signal.c @@ -858,11 +858,23 @@ static inline int rt_setup_ucontext(struct ucontext __user *uc, struct pt_regs * } static inline void __user * -get_sigframe(struct ksignal *ksig, size_t frame_size) +get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size) { - unsigned long usp = sigsp(rdusp(), ksig); + struct pt_regs *tregs = rte_regs(regs); + unsigned long usp = rdusp(); + unsigned long ssp = sigsp(usp, ksig); + + if (CPU_IS_020_OR_030 && ssp == usp && tregs->format == 0xb) { + struct frame *fmtbframe = (struct frame *) regs; + unsigned long fltp = (unsigned long) fmtbframe->un.fmtb.daddr; + + if (fltp < ssp && + (fltp >> PAGE_SHIFT) == (ssp >> PAGE_SHIFT)-1 && + (((fltp - frame_size) & -8UL) >> PAGE_SHIFT) == (fltp >> PAGE_SHIFT)) + ssp = fltp; + } - return (void __user *)((usp - frame_size) & -8UL); + return (void __user *)((ssp - frame_size) & -8UL); } static int setup_frame(struct ksignal *ksig, sigset_t *set, @@ -880,7 +892,7 @@ static int setup_frame(struct ksignal *ksig, sigset_t *set, return -EFAULT; } - frame = get_sigframe(ksig, sizeof(*frame) + fsize); + frame = get_sigframe(ksig, regs, sizeof(*frame) + fsize); if (fsize) err |= copy_to_user (frame + 1, regs + 1, fsize); @@ -952,7 +964,7 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set, return -EFAULT; } - frame = get_sigframe(ksig, sizeof(*frame)); + frame = get_sigframe(ksig, regs, sizeof(*frame)); if (fsize) err |= copy_to_user (&frame->uc.uc_extra, regs + 1, fsize); -- 2.17.1
