Re: [PATCH v4 0/3] m68k: Improved switch stack handling

Michael Schmitz
2021-07-20T22:17:19Z

Hi Eric,

On 21/07/21 8:32 am, Eric W. Biederman wrote:
>
>> diff --git a/arch/m68k/fpsp040/skeleton.S b/arch/m68k/fpsp040/skeleton.S
>> index a8f4161..6c92d38 100644
>> --- a/arch/m68k/fpsp040/skeleton.S
>> +++ b/arch/m68k/fpsp040/skeleton.S
>> @@ -502,7 +502,17 @@ in_ea:
>> .section .fixup,#alloc,#execinstr
>> .even
>> 1:
>> +
>> + SAVE_ALL_INT
>> + SAVE_SWITCH_STACK
> ^^^^^^^^^^
>
> I don't think this saves the registers in the well known fixed location
> on the stack because some registers are saved at the exception entry
> point.

The FPU exception entry points are not using the exception entry code in
head.S. These entry points are stored in the exception vector table
directly. No saving of a syscall stack frame happens there. The FPU
places its exception frame on the stack, and that is what the FPU
exception handlers use.

(If these have to call out to the generic exception handlers again, they
will build a minimal stack frame, see code in skeleton.S.)

Calling fpsp040_die() is no different from calling a syscall that may
need to have access to the full stack frame. The 'fixed location' is
just 'on the stack before calling  fpsp040_die()', again this is no
different from calling e.g. sys_fork() which does not take a pointer to
the begin of the stack frame as an argument.

I must admit I never looked at how do_exit() figures out where the stack
frame containing the saved registers is stored, I just assumed it
unwinds the stack up to the point where the caller syscall was made, and
works from there. The same strategy ought to work here.

>
> Without being saved at the well known fixed location if some process
> stops in PTRACE_EVENT_EXIT in do_exit we likely get some complete
> gibberish.
>
> That is probably safe.
>
>> jbra fpsp040_die
>> + addql #8,%sp
>> + addql #8,%sp
>> + addql #8,%sp
>> + addql #8,%sp
>> + addql #8,%sp
>> + addql #4,%sp
>> + rts
> Especially as everything after jumping to fpsp040_die does not execute.

Unless we change fpsp040_die() to call force_sig(SIGSEGV).

Cheers,

Michael

>
> Eric
>
>
>>
>> .section __ex_table,#alloc
>> .align 4