Hi Finn, Am 26.04.2024 um 13:22 schrieb Michael Schmitz:
Hi Finn, yes, that would explain that. Using a start address of badpage-4 and path '/tmp' or '/temp' in order to use either the movesw or movesb branches of the code (and force a fault on the first byte in the movesw case), I see no more Oops. Still have to test forcing the fault on the second byte of a movesw (making it a misaligned access again).
Similar tests with start address five or six bytes before the start of the unmapped page, and corresponding path length to be returned by getcwd have shown no more Oops on 030 using the attached corrected version of my patch.
Please give that some testing if you can, and (hoping it won't show any new faults on 040) I'll post another version of the series with your Tested-by added.
Cheers, Michael
From d91cf6c8d282e61e57c03e9614ed64eecce54e10 Mon Sep 17 00:00:00 2001
From: Michael Schmitz <schmitzmic@xxxxxxxxx> Date: Wed, 17 Apr 2024 08:47:55 +1200 Subject: [PATCH 1/2] m68k: Handle __generic_copy_to_user faults more carefully As mentioned by Finn Thain in his patch to improve put_user exception handling on 040, a similar problem exists on 030 processors. A moves instruction that crosses a page boundary from a mapped page into an unmapped one will cause a mid-instruction bus error exception (frame format b), with the PC pointing (usually) two instructions past the faulting movesl instruction. Our exception handling in __generic_copy_to_user only covers the instruction immediately following the faulting one. As a result, fixup_exception in send_fault_sig does not detect this case, and cause send_fault_sig to oops. Extend the exception table to cover one additional instruction beyond the moves[lwb] instructions. Tested on 68030 (Atari Falcon 030) with transfers beginning at one to six bytes offset from the end of a mapped page, followed by further bytes on an unmapped page (testcase derived from stress-ng sysbadaddr stressor by Finn Thain). Tested on 68040 (Mac Quadra) by Finn Thain. A similar problem exists in __clear_user(); modify the exception table for that function in the same way (tested by Finn Thain). Cc: Finn Thain <fthain@xxxxxxxxxxxxxx> Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> Cc: linux-m68k@xxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/all/e0f23460779e6d16e2633486ac4841790ef2aca0.1713176294.git.fthain@xxxxxxxxxxxxxx Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx> --- Changes from RFC v2: Finn Thain: - add missing extension table entries and final NOP after faults in 040 tests Changes from RFC v1: Michael Schmitz: - use extended exception table instead of additional NOPs --- arch/m68k/lib/uaccess.c | 55 ++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c index 7646e461aa62..1ad4be5f90e9 100644 --- a/arch/m68k/lib/uaccess.c +++ b/arch/m68k/lib/uaccess.c @@ -60,35 +60,42 @@ unsigned long __generic_copy_to_user(void __user *to, const void *from, asm volatile ("\n" " tst.l %0\n" - " jeq 4f\n" + " jeq 5f\n" "1: move.l (%1)+,%3\n" "2: "MOVES".l %3,(%2)+\n" "3: subq.l #1,%0\n" - " jne 1b\n" - "4: btst #1,%5\n" - " jeq 6f\n" - " move.w (%1)+,%3\n" - "5: "MOVES".w %3,(%2)+\n" - "6: btst #0,%5\n" + "4: jne 1b\n" + "5: btst #1,%5\n" " jeq 8f\n" - " move.b (%1)+,%3\n" - "7: "MOVES".b %3,(%2)+\n" - "8:\n" + "6: move.w (%1)+,%3\n" + "7: "MOVES".w %3,(%2)+\n" + "8: btst #0,%5\n" + "9: jeq 13f\n" + "10: move.b (%1)+,%3\n" + "11: "MOVES".b %3,(%2)+\n" + "12: nop\n" + "13:\n" " .section .fixup,\"ax\"\n" " .even\n" "20: lsl.l #2,%0\n" "50: add.l %5,%0\n" - " jra 8b\n" + " jra 13b\n" " .previous\n" "\n" " .section __ex_table,\"a\"\n" " .align 4\n" + " .long 1b,20b\n" " .long 2b,20b\n" " .long 3b,20b\n" - " .long 5b,50b\n" + " .long 4b,20b\n" + " .long 5b,20b\n" " .long 6b,50b\n" " .long 7b,50b\n" " .long 8b,50b\n" + " .long 9b,50b\n" + " .long 10b,50b\n" + " .long 11b,50b\n" + " .long 12b,50b\n" " .previous" : "=d" (res), "+a" (from), "+a" (to), "=&d" (tmp) : "0" (n / 4), "d" (n & 3)); @@ -107,32 +114,34 @@ unsigned long __clear_user(void __user *to, unsigned long n) asm volatile ("\n" " tst.l %0\n" - " jeq 3f\n" + " jeq 4f\n" "1: "MOVES".l %2,(%1)+\n" "2: subq.l #1,%0\n" - " jne 1b\n" - "3: btst #1,%4\n" - " jeq 5f\n" - "4: "MOVES".w %2,(%1)+\n" - "5: btst #0,%4\n" - " jeq 7f\n" - "6: "MOVES".b %2,(%1)\n" - "7:\n" + "3: jne 1b\n" + "4: btst #1,%4\n" + " jeq 6f\n" + "5: "MOVES".w %2,(%1)+\n" + "6: btst #0,%4\n" + "7: jeq 9f\n" + "8: "MOVES".b %2,(%1)\n" + "9:\n" " .section .fixup,\"ax\"\n" " .even\n" "10: lsl.l #2,%0\n" "40: add.l %4,%0\n" - " jra 7b\n" + " jra 9b\n" " .previous\n" "\n" " .section __ex_table,\"a\"\n" " .align 4\n" " .long 1b,10b\n" " .long 2b,10b\n" - " .long 4b,40b\n" + " .long 3b,10b\n" " .long 5b,40b\n" " .long 6b,40b\n" " .long 7b,40b\n" + " .long 8b,40b\n" + " .long 9b,40b\n" " .previous" : "=d" (res), "+a" (to) : "d" (0), "0" (n / 4), "d" (n & 3)); -- 2.17.1
