Hi Finn,
Am 15.04.2024 um 22:18 schrieb Finn Thain:
Running 'stress-ng --sysbadaddr -1' on my MC68040 system immediately
produces an oops:
[...]
The cause is a deliberately misaligned access in the 'bad_end_addr' test
case in the 'sysbadaddr' stressor. The location being accessed here,
0xc043dfff, was contrived to span the boundary between a r/w anonymous page
and an unmapped page. The address was then passed to the getcwd syscall
which faulted in copy_to_user().
The fault for the mapped page appears to be handled okay -- up until
do_040writeback1() called put_user() which produced a second fault due to
the unmapped page.
Michael Schmitz helpfully deciphered the oops and explained the exception
processing leading up to it.
"regs->pc does point to the PC in the format 7 frame which is the PC
the fault was detected at, but not (in case of a writeback fault)
the PC of the faulting instruction [that is, MOVES.L].
"The writeback would still cross the page boundary, and fault if the
unmapped page still isn't present. We would not see the PC of the
movesl in that case, and fail to find the PC in the exception
table."
One solution is to add a NOP instruction after the MOVES.L to flush the
pipeline and take the fault. That way, the PC value in the exception frame
becomes dependable so the exception table works.
Theoretically, there seems to be another bug in the existing code. If
the instruction following the MOVES faulted, then after the fixup,
execution would resume at the instruction which caused the fault. This
appears to be a loop. After this patch, that cannot happen.
[...]
---
arch/m68k/include/asm/uaccess.h | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/m68k/include/asm/uaccess.h b/arch/m68k/include/asm/uaccess.h
index 64914872a5c9..44e52d8323e5 100644
--- a/arch/m68k/include/asm/uaccess.h
+++ b/arch/m68k/include/asm/uaccess.h
@@ -31,11 +31,12 @@
#define __put_user_asm(inst, res, x, ptr, bwl, reg, err) \
asm volatile ("\n" \
"1: "inst"."#bwl" %2,%1\n" \
- "2:\n" \
+ "2: nop\n" \
+ "3:\n" \
" .section .fixup,\"ax\"\n" \
" .even\n" \
"10: moveq.l %3,%0\n" \
- " jra 2b\n" \
+ " jra 3b\n" \
" .previous\n" \
"\n" \
" .section __ex_table,\"a\"\n" \
@@ -53,11 +54,12 @@ do { \
asm volatile ("\n" \
"1: "inst".l %2,(%1)+\n" \
"2: "inst".l %R2,(%1)\n" \
- "3:\n" \
+ "3: nop\n" \
+ "4:\n" \
" .section .fixup,\"ax\"\n" \
" .even\n" \
"10: movel %3,%0\n" \
- " jra 3b\n" \
+ " jra 4b\n" \
" .previous\n" \
"\n" \
" .section __ex_table,\"a\"\n" \
Extensively tested on 68030, too (where there isn't a writeback but
put_user can still fault in the same context), and after some review and
testing I'm satisfed adding NOPs is the only solution, so:
Tested-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
Reviewed-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
