Re: [PATCH v2] m68k: kernel/traps.c - only force 030 bus error if PC not in exception table

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Geert,

On 7/03/23 20:44, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Tue, Mar 7, 2023 at 1:28 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> On 7/03/23 02:03, Geert Uytterhoeven wrote:
>>> On Wed, Mar 1, 2023 at 3:13 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>>>> __get_kernel_nofault() does copy data in supervisor mode when
>>>> forcing a task backtrace log through /proc/sysrq_trigger.
>>>> This is expected cause a bus error exception on e.g. NULL
>>>> pointer dereferencing when logging a kernel task has no
>>>> workqueue associated. This bus error ought to be ignored.
>>>>
>>>> Our 030 bus error handler is ill equipped to deal with this:
>>>>
>>>> Whenever ssw indicates a kernel mode access on a data fault,
>>>> we don't even attempt to handle the fault and instead always
>>>> send a SEGV signal (or panic). As a result, the check
>>>> for exception handling at the fault PC (buried in
>>>> send_sig_fault() which gets called from do_page_fault()
>>>> eventually) is never used.
>>>>
>>>> In contrast, both 040 and 060 access error handlers do not
>>>> care whether a fault happened on supervisor mode access,
>>>> and will call do_page_fault() on those, ultimately honoring
>>>> the exception table.
>>>>
>>>> Add a check in bus_error030 to call do_page_fault() in case
>>>> we do have an entry for the fault PC in our exception table.
>>>>
>>>> I had attempted a fix for this earlier in 2019 that did rely
>>>> on testing pagefault_disabled() (see link below) to achieve
>>>> the same thing, but this patch should be more generic.
>>>>
>>>> Tested on 030 Atari Falcon.
>>>>
>>>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>>>> Reported-by: Eero Tamminen <oak@helsinkinet.fi>
>>>> CC: Eero Tamminen <oak@helsinkinet.fi>
>>>> CC: Finn Thain <fthain@linux-m68k.org>
>>>> CC: Andreas Schwab <schwab@linux-m68k.org>
>>>> CC: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> Link: https://lore.kernel.org/r/alpine.LNX.2.21.1904091023540.25@nippy.intranet
>>>> Link: https://lore.kernel.org/r/63130691-1984-c423-c1f2-73bfd8d3dcd3@gmail.com
>>>> --
>>>>
>>>> Changes from v1:
>>>>
>>>> - add comment
>>>> - reword commit message
>>>> - add link to old patch as well (stick to lore.kernel.org
>>>>     even though these links are currently not functional)
>>> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> i.e. will queue as a fix in the m68k for-v6.3 branch.
>> Thanks - I neglected to search far enough back to find where the context
>> of this diff originates, but upon a closer look, it appears to be the
>> well-known commit 1da177e4c3f4 (start of git history). Do you think this
>> is worth a backport to stable?
> I guess it will be auto-backported to stable once it has reached
> upstream, together with the two other fixes.

Fair enough - I'll check in a month or so and remind the stable 
maintainers if need be.

Cheers,

     Michael


>
> Gr{oetje,eeting}s,
>
>                          Geert
>