* [PATCH v3 0/2] m68k uaccess fault handling fixes @ 2024-04-27 8:48 Michael Schmitz 2024-04-27 8:48 ` [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz 2024-04-27 8:48 ` [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz 0 siblings, 2 replies; 9+ messages in thread From: Michael Schmitz @ 2024-04-27 8:48 UTC (permalink / raw) To: linux-m68k, geert; +Cc: gerg Version 4 of fixes for uaccess fault handling on 68030, tested on 68030 and 68040. Patch 1 has additional exception table entries as well as a final NOP added - evidently, in some cases the fault PC points more than a single instruction past the moves that caused the fault. Patch 2 is unchanged from RFC v2. These patches would still benefit from testing on 68060 and Coldfire. Looking at access_error060, I would not expect the 68060 to behave much different from the 68040 and in particular, addition of exception table entries to not cause any regression. Less confident for Coldfire. Cheers, Michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-27 8:48 [PATCH v3 0/2] m68k uaccess fault handling fixes Michael Schmitz @ 2024-04-27 8:48 ` Michael Schmitz 2024-04-27 9:34 ` Finn Thain 2024-04-27 8:48 ` [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz 1 sibling, 1 reply; 9+ messages in thread From: Michael Schmitz @ 2024-04-27 8:48 UTC (permalink / raw) To: linux-m68k, geert; +Cc: gerg, Michael Schmitz, Finn Thain, linux-m68k 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) and 68030 (Mac IIci) by Finn Thain. A similar problem is present in __clear_user(); modify the exception table for that function in the same way (tested by Finn Thain). Cc: Finn Thain <fthain@linux-m68k.org> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Cc: linux-m68k@lists.linux-m68k.org Tested-by: Finn Thain <fthain@linux-m68k.org> Link: https://lore.kernel.org/all/e0f23460779e6d16e2633486ac4841790ef2aca0.1713176294.git.fthain@linux-m68k.org Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- Changes from RFC v2: Finn Thain: - add missing extension table entries and final NOP in __generic_copy_to_user faults in 040 tests Michael Schmitz: - add yet another exception table entry in __clear_user Changes from RFC v1: Michael Schmitz: - use extended exception table instead of additional NOPs --- arch/m68k/lib/uaccess.c | 56 ++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c index 86b6fed5151c..e3ed893047f8 100644 --- a/arch/m68k/lib/uaccess.c +++ b/arch/m68k/lib/uaccess.c @@ -62,35 +62,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)); @@ -109,32 +116,35 @@ 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 4b,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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-27 8:48 ` [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz @ 2024-04-27 9:34 ` Finn Thain 2024-04-27 23:16 ` Michael Schmitz 0 siblings, 1 reply; 9+ messages in thread From: Finn Thain @ 2024-04-27 9:34 UTC (permalink / raw) To: Michael Schmitz; +Cc: linux-m68k, geert, gerg, linux-m68k On Sat, 27 Apr 2024, Michael Schmitz wrote: > > diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c > index 86b6fed5151c..e3ed893047f8 100644 > --- a/arch/m68k/lib/uaccess.c > +++ b/arch/m68k/lib/uaccess.c > @@ -62,35 +62,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" I think a fault at 6b has to get fixed up at 20b. > " .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)); > @@ -109,32 +116,35 @@ 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" Should we put a NOP here to avoid having the unknown next instruction (label 9) in the exception table? We can't actually fix up a fault there unless by chance it was the MOVES that caused it. > " .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 4b,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)); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-27 9:34 ` Finn Thain @ 2024-04-27 23:16 ` Michael Schmitz 2024-04-28 3:28 ` Finn Thain 0 siblings, 1 reply; 9+ messages in thread From: Michael Schmitz @ 2024-04-27 23:16 UTC (permalink / raw) To: Finn Thain; +Cc: linux-m68k, geert, gerg, linux-m68k Hi Finn, thanks for your review! Am 27.04.2024 um 21:34 schrieb Finn Thain: > > On Sat, 27 Apr 2024, Michael Schmitz wrote: > >> >> diff --git a/arch/m68k/lib/uaccess.c b/arch/m68k/lib/uaccess.c >> index 86b6fed5151c..e3ed893047f8 100644 >> --- a/arch/m68k/lib/uaccess.c >> +++ b/arch/m68k/lib/uaccess.c >> @@ -62,35 +62,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" > > I think a fault at 6b has to get fixed up at 20b. In my tests, the fault seen there had been caused by the movesl in the section above, hence the fixup at 20b. But your comment has me thinking about a side effect of this change: in case the movew at label 6 were to fail, that fault was masked by our exception handling. We'd silently ignore a kernel bus error there. On the other hand, we'd probably see a fault there manifest with a PC corresponding to the movesw or btest instruction rather than the movew instruction. These instructions have always been in the exception table, so potentially missing a kernel bus error there has always been an issue. The only way I see that would avoid having to place this instruction (and the moveb later) into the exception table is to place a NOP after each of the movesl and movesw sections. Shouldn't have too much of a performance impact as long as it's not placed inside the movesl loop. > >> " .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)); >> @@ -109,32 +116,35 @@ 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" > > Should we put a NOP here to avoid having the unknown next instruction > (label 9) in the exception table? We can't actually fix up a fault there > unless by chance it was the MOVES that caused it. The movesb does not have the potential to cause a misaligned access, and I believe it is for that reason that a NOP there is not required (it probably isn't for __generic_copy_to_user either). My tests on 030 at least have confirmed that - selecting path length and start offset such that the movesb is the first instruction hitting the unmapped page has never produced an Ooops. As to the unknown instructions following the final exception label: These functions, though they contain inline assembly code are not further inlined, and the instructions following the inline assembly are simple boilerplate register restore stack saves that ought not to fault (an invalid stack pointer would have faulted on function entry, if at all). On balance, I am confident the code is correct as-is. You (and in particular, Geert) may argue though that the NOP approach follows the principle of least surprises, and can be considered safe to apply without further testing on 68060 and Coldfire. Cheers, Michael > >> " .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 4b,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)); >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-27 23:16 ` Michael Schmitz @ 2024-04-28 3:28 ` Finn Thain 2024-04-28 3:51 ` Michael Schmitz 0 siblings, 1 reply; 9+ messages in thread From: Finn Thain @ 2024-04-28 3:28 UTC (permalink / raw) To: Michael Schmitz; +Cc: linux-m68k, geert, gerg, linux-m68k On Sun, 28 Apr 2024, Michael Schmitz wrote: > In my tests, the fault seen there had been caused by the movesl in the > section above, hence the fixup at 20b. > Well, that was my point. A fault at MOVE.W should get fixed up at 20b, not 50b. So the patch is wrong, isn't it? > > > > Should we put a NOP here to avoid having the unknown next instruction > > (label 9) in the exception table? We can't actually fix up a fault > > there unless by chance it was the MOVES that caused it. > > ... As to the unknown instructions following the final exception label: > These functions, though they contain inline assembly code are not > further inlined, and the instructions following the inline assembly are > simple boilerplate register restore stack saves that ought not to fault > (an invalid stack pointer would have faulted on function entry, if at > all). > But that's just luck. IMHO, the asm is a foot-gun even if it has not gone off yet. > On balance, I am confident the code is correct as-is. You (and in > particular, Geert) may argue though that the NOP approach follows the > principle of least surprises, and can be considered safe to apply > without further testing on 68060 and Coldfire. > If you're saying that your patch addresses a different bug, fair enough. All I'm saying is that, since you're adding the NOP anyway, you could make better use of it. Anyway, like you, I am keen to hear from others regarding the API issue. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-28 3:28 ` Finn Thain @ 2024-04-28 3:51 ` Michael Schmitz 2024-04-28 8:03 ` Finn Thain 0 siblings, 1 reply; 9+ messages in thread From: Michael Schmitz @ 2024-04-28 3:51 UTC (permalink / raw) To: Finn Thain; +Cc: linux-m68k, geert, gerg, linux-m68k Hi Finn, Am 28.04.2024 um 15:28 schrieb Finn Thain: > On Sun, 28 Apr 2024, Michael Schmitz wrote: > >> In my tests, the fault seen there had been caused by the movesl in the >> section above, hence the fixup at 20b. >> > > Well, that was my point. A fault at MOVE.W should get fixed up at 20b, not > 50b. So the patch is wrong, isn't it? You are correct there - the fault may have happened while there still were longwords to transfer, even though the fault PC is after the loop. The residual from the loop must be taken into account. I'll fix that. >>> >>> Should we put a NOP here to avoid having the unknown next instruction >>> (label 9) in the exception table? We can't actually fix up a fault >>> there unless by chance it was the MOVES that caused it. >> >> ... As to the unknown instructions following the final exception label: >> These functions, though they contain inline assembly code are not >> further inlined, and the instructions following the inline assembly are >> simple boilerplate register restore stack saves that ought not to fault >> (an invalid stack pointer would have faulted on function entry, if at >> all). >> > > But that's just luck. IMHO, the asm is a foot-gun even if it has not gone > off yet. If the compiler were to inline the entire function, that could happen, yes. So for that reason, the NOP would be safer. > >> On balance, I am confident the code is correct as-is. You (and in >> particular, Geert) may argue though that the NOP approach follows the >> principle of least surprises, and can be considered safe to apply >> without further testing on 68060 and Coldfire. >> > > If you're saying that your patch addresses a different bug, fair enough. Nah, I was just speculating that instead of bloating the exception table (and risking to miss a bad kernel address as source of the transfer), addition of NOPs might be considered the safer option. > > All I'm saying is that, since you're adding the NOP anyway, you could make > better use of it. I already am, in __generic_copy_to_user - there is no exception table entry for the jump label at the end, so no loop. Since I have to fix patch 1 anyway, I'll add the NOP at the end of __clear_user and have the exception table end with that NOP. > Anyway, like you, I am keen to hear from others regarding the API issue. Too true ... Cheers, Michael ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully 2024-04-28 3:51 ` Michael Schmitz @ 2024-04-28 8:03 ` Finn Thain 0 siblings, 0 replies; 9+ messages in thread From: Finn Thain @ 2024-04-28 8:03 UTC (permalink / raw) To: Michael Schmitz; +Cc: linux-m68k, geert, gerg, linux-m68k On Sun, 28 Apr 2024, Michael Schmitz wrote: > > > > All I'm saying is that, since you're adding the NOP anyway, you could > > make better use of it. > > I already am, in __generic_copy_to_user - there is no exception table > entry for the jump label at the end, so no loop. > Right you are. I did get __generic_copy_to_user and __clear_user mixed up there. > Since I have to fix patch 1 anyway, I'll add the NOP at the end of > __clear_user and have the exception table end with that NOP. > Well, I'll leave that up to you and Geert. I'm ambivalent about that one. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling 2024-04-27 8:48 [PATCH v3 0/2] m68k uaccess fault handling fixes Michael Schmitz 2024-04-27 8:48 ` [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz @ 2024-04-27 8:48 ` Michael Schmitz 2024-04-27 9:24 ` Finn Thain 1 sibling, 1 reply; 9+ messages in thread From: Michael Schmitz @ 2024-04-27 8:48 UTC (permalink / raw) To: linux-m68k, geert; +Cc: gerg, Michael Schmitz, Finn Thain A problem similar to that reported for __put_user_asm and __generic_copy_to_user is also present in __constant_copy_to_user_asm. Address the problem by extending the exception table to cover two instructions past each moves instruction, and adding a single NOP at the very end to catch faults on the final instruction (which is not guaranteed to be a movesb!). Tested on 68030 (Atari Falcon 030) with a transfer beginning at a single byte at the end of a mapped page followed by seven more bytes on an unmapped page (testcase derived from stress-ng sysbadaddr stressor by Finn Thain and modified to use the llseek syscall). Cc: Finn Thain <fthain@linux-m68k.org> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Link: https://lore.kernel.org/all/e0f23460779e6d16e2633486ac4841790ef2aca0.1713176294.git.fthain@linux-m68k.org Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- arch/m68k/include/asm/uaccess.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h index 44e52d8323e5..f1f4b62d6f69 100644 --- a/arch/m68k/include/asm/uaccess.h +++ b/arch/m68k/include/asm/uaccess.h @@ -288,10 +288,11 @@ __constant_copy_from_user(void *to, const void __user *from, unsigned long n) "21: "MOVES"."#s2" %3,(%1)+\n" \ "22:\n" \ " .ifnc \""#s3"\",\"\"\n" \ - " move."#s3" (%2)+,%3\n" \ - "31: "MOVES"."#s3" %3,(%1)+\n" \ - "32:\n" \ + "31: move."#s3" (%2)+,%3\n" \ + "32: "MOVES"."#s3" %3,(%1)+\n" \ + "33:\n" \ " .endif\n" \ + "34: nop\n" \ "4:\n" \ "\n" \ " .section __ex_table,\"a\"\n" \ @@ -303,7 +304,9 @@ __constant_copy_from_user(void *to, const void __user *from, unsigned long n) " .ifnc \""#s3"\",\"\"\n" \ " .long 31b,5f\n" \ " .long 32b,5f\n" \ + " .long 33b,5f\n" \ " .endif\n" \ + " .long 34b,5f\n" \ " .previous\n" \ "\n" \ " .section .fixup,\"ax\"\n" \ -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling 2024-04-27 8:48 ` [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz @ 2024-04-27 9:24 ` Finn Thain 0 siblings, 0 replies; 9+ messages in thread From: Finn Thain @ 2024-04-27 9:24 UTC (permalink / raw) To: Michael Schmitz; +Cc: linux-m68k, geert, gerg On Sat, 27 Apr 2024, Michael Schmitz wrote: > A problem similar to that reported for __put_user_asm and > __generic_copy_to_user is also present in > __constant_copy_to_user_asm. > > Address the problem by extending the exception table to cover > two instructions past each moves instruction, and adding a > single NOP at the very end to catch faults on the final > instruction (which is not guaranteed to be a movesb!). > > Tested on 68030 (Atari Falcon 030) with a transfer beginning > at a single byte at the end of a mapped page followed by > seven more bytes on an unmapped page (testcase derived from > stress-ng sysbadaddr stressor by Finn Thain and modified to > use the llseek syscall). > > Cc: Finn Thain <fthain@linux-m68k.org> > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Link: https://lore.kernel.org/all/e0f23460779e6d16e2633486ac4841790ef2aca0.1713176294.git.fthain@linux-m68k.org > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> Tested-by: Finn Thain <fthain@linux-m68k.org> > --- > arch/m68k/include/asm/uaccess.h | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h > index 44e52d8323e5..f1f4b62d6f69 100644 > --- a/arch/m68k/include/asm/uaccess.h > +++ b/arch/m68k/include/asm/uaccess.h > @@ -288,10 +288,11 @@ __constant_copy_from_user(void *to, const void __user *from, unsigned long n) > "21: "MOVES"."#s2" %3,(%1)+\n" \ > "22:\n" \ > " .ifnc \""#s3"\",\"\"\n" \ > - " move."#s3" (%2)+,%3\n" \ > - "31: "MOVES"."#s3" %3,(%1)+\n" \ > - "32:\n" \ > + "31: move."#s3" (%2)+,%3\n" \ > + "32: "MOVES"."#s3" %3,(%1)+\n" \ > + "33:\n" \ > " .endif\n" \ > + "34: nop\n" \ > "4:\n" \ > "\n" \ > " .section __ex_table,\"a\"\n" \ > @@ -303,7 +304,9 @@ __constant_copy_from_user(void *to, const void __user *from, unsigned long n) > " .ifnc \""#s3"\",\"\"\n" \ > " .long 31b,5f\n" \ > " .long 32b,5f\n" \ > + " .long 33b,5f\n" \ > " .endif\n" \ > + " .long 34b,5f\n" \ > " .previous\n" \ > "\n" \ > " .section .fixup,\"ax\"\n" \ > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-04-28 8:03 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-27 8:48 [PATCH v3 0/2] m68k uaccess fault handling fixes Michael Schmitz 2024-04-27 8:48 ` [PATCH v3 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz 2024-04-27 9:34 ` Finn Thain 2024-04-27 23:16 ` Michael Schmitz 2024-04-28 3:28 ` Finn Thain 2024-04-28 3:51 ` Michael Schmitz 2024-04-28 8:03 ` Finn Thain 2024-04-27 8:48 ` [PATCH v3 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz 2024-04-27 9:24 ` Finn Thain
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.