From: Michael Schmitz <schmitzmic@gmail.com>
To: Finn Thain <fthain@linux-m68k.org>
Cc: geert@linux-m68k.org, linux-m68k@lists.linux-m68k.org
Subject: Re: [PATCH RFC v2 1/2] m68k: Handle __generic_copy_to_user faults more carefully
Date: Fri, 26 Apr 2024 19:10:10 +1200 [thread overview]
Message-ID: <fbaaf75c-c4e9-7fee-bb8d-9fd0fd25b197@gmail.com> (raw)
In-Reply-To: <1b49c8a1-c753-428d-b526-06b6eb3e551c@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
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
[-- Attachment #2: 0001-m68k-Handle-__generic_copy_to_user-faults-more-caref.patch --]
[-- Type: text/x-diff, Size: 4378 bytes --]
From d91cf6c8d282e61e57c03e9614ed64eecce54e10 Mon Sep 17 00:00:00 2001
From: Michael Schmitz <schmitzmic@gmail.com>
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@linux-m68k.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-m68k@lists.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 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
next prev parent reply other threads:[~2024-04-26 7:10 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 2:29 [PATCH RFC v2 0/2] m68k uaccess fault handling fixes Michael Schmitz
2024-04-22 2:29 ` [PATCH RFC v2 1/2] m68k: Handle __generic_copy_to_user faults more carefully Michael Schmitz
2024-04-25 4:16 ` Finn Thain
2024-04-25 5:32 ` Michael Schmitz
2024-04-25 6:32 ` Finn Thain
2024-04-25 7:52 ` Michael Schmitz
2024-04-25 5:45 ` Michael Schmitz
2024-04-25 6:47 ` Finn Thain
2024-04-25 7:43 ` Michael Schmitz
2024-04-25 8:20 ` Michael Schmitz
2024-04-25 19:15 ` Michael Schmitz
2024-04-26 1:00 ` Finn Thain
2024-04-26 1:22 ` Michael Schmitz
2024-04-26 7:10 ` Michael Schmitz [this message]
2024-04-26 7:57 ` Finn Thain
2024-04-26 8:31 ` Michael Schmitz
2024-04-26 7:58 ` Finn Thain
2024-04-27 1:44 ` Finn Thain
2024-04-27 4:41 ` Michael Schmitz
2024-04-22 2:29 ` [PATCH RFC v2 2/2] m68k: improve __constant_copy_to_user_asm() fault handling Michael Schmitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fbaaf75c-c4e9-7fee-bb8d-9fd0fd25b197@gmail.com \
--to=schmitzmic@gmail.com \
--cc=fthain@linux-m68k.org \
--cc=geert@linux-m68k.org \
--cc=linux-m68k@lists.linux-m68k.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.