Re: stack smashing detected

Michael Schmitz
2023-02-09T03:41:54Z

[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]

Hi Stan,

Am 08.02.2023 um 11:58 schrieb Michael Schmitz:
> Thanks Stan,
>
> On 8/02/23 08:37, Stan Johnson wrote:
>> Hi Michael,
>>
>> On 2/5/23 3:19 PM, Michael Schmitz wrote:
>>> ...
>>>
>>> Seeing Finn's report that Al Viro's VM_FAULT_RETRY fix may have solved
>>> his task corruption troubles on 040, I just noticed that I probably
>>> misunderstood how Al's patch works.
>>>
>>> Botching up a fault retry and carrying on may well leave the page tables
>>> in a state where some later access could go to the wrong page and
>>> manifest as user space corruption. Could you try Al's patch 4 (m68k: fix
>>> livelock in uaccess) to see if this helps?
>>> ...
>> ok, this appears to be the patch:
>>
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>> ---
>> 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)
>
> That's correct.
>
> Your results show improvement but the problem does not entirely go away.
>
> Looking at differences between 030 and 040/040 fault handling, it
> appears only 030 handles faults corrected by exception tables (such as
> used in uaccess macros) special, i.e. aborting bus error processing
> while 040 and 060 carry on in the fault handler.
>
> I wonder if that's the main difference between 030 and 040 behaviour?

Following the 040 code a bit further, I suspect that happens in the 040
writeback handler, so this may be a red herring.

> I'll try and log such accesses caught by exception tables on 030 to see
> if they are rare enough to allow adding a kernel log message...

Looks like this kind of event is rare enough to not trigger in a normal
boot on my 030. Please give the attached patch a try so we can confirm
(or rule out) that user space access faults from kernel mode are to
blame for your stack smashes.

Cheers,

Michael

> Cheers,
>
> Michael
>
>

[-- Attachment #2: 0001-m68k-debug-exception-handling-data-faults-on-030.patch --]
[-- Type: text/x-diff, Size: 1333 bytes --]

From a55467a02b66addca6f74fc32b473bc077cb34b2 Mon Sep 17 00:00:00 2001
From: Michael Schmitz <schmitzmic@gmail.com>
Date: Thu, 9 Feb 2023 14:39:35 +1300
Subject: [PATCH] m68k: debug exception handling data faults on 030

030 faults handled by exception tables are just silently ignored - see how
many of these do happen in practice, and if they are related to 'stack
smashing' faults.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---
arch/m68k/kernel/traps.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
index 5c8cba0efc63..b3cef760f7e8 100644
--- a/arch/m68k/kernel/traps.c
+++ b/arch/m68k/kernel/traps.c
@@ -554,8 +554,13 @@ static inline void bus_error030 (struct frame *fp)
}
/* Don't try to do anything further if an exception was
handled. */
- if (do_page_fault (&fp->ptregs, addr, errorcode) < 0)
+ if (do_page_fault (&fp->ptregs, addr, errorcode) < 0) {
+ pr_err("Exception handled for data %s fault at %#010lx in %s (pc=%#lx)\n",
+ ssw & RW ? "read" : "write",
+ fp->un.fmtb.daddr,
+ space_names[ssw & DFC], fp->ptregs.pc);
return;
+ }
} else if (!(mmusr & MMU_I)) {
/* probably a 020 cas fault */
if (!(ssw & RM) && send_fault_sig(&fp->ptregs) > 0)
--
2.17.1