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

[Michael Schmitz <schmitzmic@gmail.com>]

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