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

Eric W. Biederman
2021-07-17T18:52:52Z

Michael Schmitz <schmitzmic@gmail.com> writes:

> Am 16.07.2021 um 11:10 schrieb Michael Schmitz:
>> Eric,
>>
>> On 16/07/21 1:29 am, Eric W. Biederman wrote:
>>>
>>> I have been digging into this some more and I have found one place
>>> that I am having a challenge dealing with.
>>>
>>> In arch/m68k/fpsp040/skeleton.S there is an assembly version of
>>> copy_from_user that calls fpsp040_die when the bytes can not be read.
>>>
>>> Now fpsp040_die is just:
>>>
>>> /*
>>> * This function is called if an error occur while accessing
>>> * user-space from the fpsp040 code.
>>> */
>>> asmlinkage void fpsp040_die(void)
>>> {
>>> do_exit(SIGSEGV);
>>> }
>>> The problem here is the instruction emulation performed in the fpsp040
>>> code performs a very minimal saving of registers. I don't think even
>>> the normal system call entry point registers that are saved are present
>>> at that point.
>>>
>>> Is there any chance you can help me figure out how to get a stack frame
>>> with all of the registers present before fpsp040_die is called?
>>
>> I suppose adding the following code (untested) to entry.S:
>>
>> ENTRY(fpsp040_die)
>> SAVE_ALL_INT
>> jbsr fpsp040_die_c
>> jra ret_from_exception
>>
>> along with renaming above C entry point to fpsp040_die_c would add the
>> basic saved registers, but these would not necessarily reflect the state
>> of the processor when the fpsp040 trap was called. Is that what you're
>> after?
>
> I should have looked more closely at skeleton.S - most FPU exceptions
> handled there call trap_c the same way as is done for generic traps,
> i.e. SAVE_ALL_INT before, ret_from_exception after.
>
> Instead of adding code to entry.S, much better to add it in
> skeleton.S. I'll try to come up with a way to test this code path
> (calling fpsp040_die from the dz exception hander seems much the
> easiest way) to make sure this doesn't have side effects.
>
> Does do_exit() ever return?

No. The function do_exit never returns.

If it is not too much difficulty I would be in favor of having the code
do force_sigsegv(SIGSEGV), instead of calling do_exit directly.

Looking at that code I have not been able to figure out the call paths
that get into skeleton.S. I am not certain saving all of the registers
on an the exceptions that reach there make sense. In practice I suspect
taking an exception is much more expensive than saving the registers so it
might not make any difference. But this definitely looks like code that
is performance sensitive.

My sense when I was reading through skeleton.S was just one or two
registers were saved before the instruction emulation was called.

Eric