Re: [PATCH 04/10] m68k: fix livelock in uaccess

Finn Thain
2023-02-05T06:16:40Z

Hello Al,

On Tue, 31 Jan 2023, Al Viro wrote:

> m68k equivalent of 26178ec11ef3 "x86: mm: consolidate VM_FAULT_RETRY
> handling" If e.g. get_user() triggers a page fault and a fatal signal is
> caught, we might end up with handle_mm_fault() returning VM_FAULT_RETRY
> and not doing anything to page tables. In such case we must *not*
> return to the faulting insn - that would repeat the entire thing without
> making any progress; what we need instead is to treat that as failed
> (user) memory access.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

That could be a bug I was chasing back in 2021 but never found. The mmap
stressors in stress-ng were triggering a crash on a Mac Quadras, though
only rarely. Sometimes it would run all day without a failure.

Last year when I started using GCC 12 to build the kernel, I saw the same
workload fail again but the failure mode had become a silent hang/livelock
instead of the oopses I got with GCC 6.

When I press the NMI button after the livelock I always see
do_page_fault() in the backtrace. So I've been testing your patch. I've
been running the same stress-ng reproducer for about 12 hours now with no
failures which looks promising.

In case that stress-ng testing is of use:
Tested-by: Finn Thain <fthain@linux-m68k.org>

BTW, how did you identify that bug in do_page_fault()? If its the same bug
I was chasing, it could be an old one. The stress-ng logs I collected last
year include a crash from a v4.14 build.

> ---
> arch/m68k/mm/fault.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c
> index 4d2837eb3e2a..228128e45c67 100644
> --- a/arch/m68k/mm/fault.c
> +++ b/arch/m68k/mm/fault.c
> @@ -138,8 +138,11 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
> fault = handle_mm_fault(vma, address, flags, regs);
> pr_debug("handle_mm_fault returns %x\n", fault);
>
> - if (fault_signal_pending(fault, regs))
> + if (fault_signal_pending(fault, regs)) {
> + if (!user_mode(regs))
> + goto no_context;
> return 0;
> + }
>
> /* The fault is fully completed (including releasing mmap lock) */
> if (fault & VM_FAULT_COMPLETED)