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

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

[Michael Schmitz]

__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)
---
 arch/m68k/kernel/traps.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
index 5c8cba0efc63..a700807c9b6d 100644
--- a/arch/m68k/kernel/traps.c
+++ b/arch/m68k/kernel/traps.c
@@ -30,6 +30,7 @@
 #include <linux/init.h>
 #include <linux/ptrace.h>
 #include <linux/kallsyms.h>
+#include <linux/extable.h>
 
 #include <asm/setup.h>
 #include <asm/fpu.h>
@@ -545,7 +546,8 @@ static inline void bus_error030 (struct frame *fp)
 			errorcode |= 2;
 
 		if (mmusr & (MMU_I | MMU_WP)) {
-			if (ssw & 4) {
+			/* We might have an exception table for this PC */
+			if (ssw & 4 && !search_exception_tables(fp->ptregs.pc)) {
 				pr_err("Data %s fault at %#010lx in %s (pc=%#lx)\n",
 				       ssw & RW ? "read" : "write",
 				       fp->un.fmtb.daddr,
-- 
2.17.1

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

[Geert Uytterhoeven]

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.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Michael Schmitz]

Hi Geert,

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?

Cheers,

     Michael

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

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

[Geert Uytterhoeven]

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.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Michael Schmitz]

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
>