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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).