From: Michael Schmitz <schmitzmic@gmail.com>
To: Finn Thain <fthain@linux-m68k.org>
Cc: linux-m68k@lists.linux-m68k.org
Subject: Re: [RFC PATCH] m68k: Handle put_user faults more carefully
Date: Sat, 27 Apr 2024 20:56:35 +1200 [thread overview]
Message-ID: <cb7b2221-6ee2-ced0-55e4-2326c695d237@gmail.com> (raw)
In-Reply-To: <e0f23460779e6d16e2633486ac4841790ef2aca0.1713176294.git.fthain@linux-m68k.org>
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@gmail.com>
Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>
prev parent reply other threads:[~2024-04-27 8:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 10:18 [RFC PATCH] m68k: Handle put_user faults more carefully Finn Thain
2024-04-24 4:48 ` Michael Schmitz
2024-04-27 8:56 ` Michael Schmitz [this message]
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=cb7b2221-6ee2-ced0-55e4-2326c695d237@gmail.com \
--to=schmitzmic@gmail.com \
--cc=fthain@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.