Re: [RFC][PATCH] signal/m68k: Use force_sigsegv(SIGSEGV) in fpsp040_die

Michael Schmitz
2021-07-26T20:29:42Z

Hi Eric,

looks good to me!

On 27/07/21 7:36 am, Eric W. Biederman wrote:
> In the fpsp040 code when copyin or copyout fails call
> force_sigsegv(SIGSEGV) instead of do_exit(SIGSEGV).
>
> This solves a couple of problems. Because do_exit embeds the ptrace
> stop PTRACE_EVENT_EXIT a complete stack frame needs to be present for
> that to work correctly. There is always the information needed for a
> ptrace stop where get_signal is called. So exiting with a signal
> solves the ptrace issue.
>
> Further exiting with a signal ensures that all of the threads in a
> process are killed not just the thread that malfunctioned. Which
> avoids confusing userspace.
>
> To make force_sigsegv(SIGSEGV) work in fpsp040_die modify the code to
> save all of the registers and jump to ret_from_exception (which
> ultimately calls get_signal) after fpsp040_die returns.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>
> Can someone please check my m68k assembly changes?
>
> I think I have them correct, and the code assembles but I don't
> understand the fine points of when the different branch instructions
> should be used.

Since the exception handler ends up in a different text section from the
actual code, long offsets are in use for jumps there.

According to the gas manual (and pointed out by Andreas just now), 'jmp'
is used only for longword offsets on 68000/010. Use 'bral' for 020 etc.
The pseudo-ops 'jra' or 'jbra' will pick the correct version (shortest
offset possible). Similar for 'jbsr' when calling a subroutine.

1:
-    jbra    fpsp040_die
+    jbsr    fpsp040_die
+    jbra    .Lnotkern

would be the most generic version to write this (but as this code is
never used on 68000, 'brsl' and 'jbra' is perfectly OK).

Cheers,

Michael

>
> arch/m68k/fpsp040/skeleton.S | 3 ++-
> arch/m68k/kernel/traps.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/m68k/fpsp040/skeleton.S b/arch/m68k/fpsp040/skeleton.S
> index a8f41615d94a..ec767523c012 100644
> --- a/arch/m68k/fpsp040/skeleton.S
> +++ b/arch/m68k/fpsp040/skeleton.S
> @@ -502,7 +502,8 @@ in_ea:
> .section .fixup,#alloc,#execinstr
> .even
> 1:
> - jbra fpsp040_die
> + bsrl fpsp040_die
> + jmp .Lnotkern
>
> .section __ex_table,#alloc
> .align 4
> diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
> index 9e1261462bcc..5b19fcdcd69e 100644
> --- a/arch/m68k/kernel/traps.c
> +++ b/arch/m68k/kernel/traps.c
> @@ -1150,7 +1150,7 @@ asmlinkage void set_esp0(unsigned long ssp)
> */
> asmlinkage void fpsp040_die(void)
> {
> - do_exit(SIGSEGV);
> + force_sigsegv(SIGSEGV);
> }
>
> #ifdef CONFIG_M68KFPU_EMU