Re: [PATCH v4 3/3] m68k: track syscalls being traced with shallow user context stack

Michael Schmitz
2021-07-25T20:48:40Z

Hi Geert,

thanks for the feedback!

As far as I understand, Eric's 'refactor exit()' patch series has
obsoleted this band-aid fix of mine. The last remnant of code using
do_exit() is our fpsp040 copyin/copyout exception handling, and there's
another patch in testing for that. (I'd need access to a 040 hardware
setup to properly test that one, but that's a different matter.)

Eric, Andreas - please correct me if I'm wrong (again).

Just out of interest - what would be the correct way to set/clear a
single bit on Coldfire? Add/subtract the 1<<bit value?

Cheers,

Michael

On 25/07/21 10:05 pm, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Wed, Jun 23, 2021 at 2:21 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Add 'status' field to thread_info struct to hold syscall trace
>> status info.
>>
>> Set flag bit in thread_info->status at syscall trace entry, clear
>> flag bit on trace exit.
>>
>> Set another flag bit on entering syscall where the full stack
>> frame has been saved. These flags can be checked whenever a
>> syscall calls ptrace_stop().
>>
>> Check flag bits in get_reg()/put_reg() and prevent access to
>> registers that are saved on the switch stack, in case the
>> syscall did not actually save these registers on the switch
>> stack.
>>
>> Tested on ARAnyM only - boots and survives running strace on a
>> binary, nothing fancy.
>>
>> CC: Eric W. Biederman <ebiederm@xmission.com>
>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>> CC: Andreas Schwab <schwab@linux-m68k.org>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> Thanks for your patch!
>
>> --- a/arch/m68k/kernel/entry.S
>> +++ b/arch/m68k/kernel/entry.S
>> @@ -51,75 +51,115 @@
>>
>> .text
>> ENTRY(__sys_fork)
>> + movel %curptr@(TASK_STACK),%a1
>> + orb #TIS_SWITCH_STACK, %a1@(TINFO_STATUS+3)
> This doesn't work on Coldfire:
>
> arch/m68k/kernel/entry.S:55: Error: invalid instruction for this
> architecture; needs 68000 or higher (68000 [68ec000, 68hc000, 68hc001,
> 68008, 68302, 68306, 68307, 68322, 68356], 68010, 68020 [68k,
> 68ec020], 68030 [68ec030], 68040 [68ec040], 68060 [68ec060], cpu32
> [68330, 68331, 68332,
> 68333, 68334, 68336, 68340, 68341, 68349, 68360], fidoa [fido]) --
> statement `orb #(1<<1),%a1@(16+3)' ignored
>
>> SAVE_SWITCH_STACK
>> jbsr sys_fork
>> lea %sp@(24),%sp
>> + movel %curptr@(TASK_STACK),%a1
>> + andb #TIS_NO_SWITCH_STACK, %a1@(TINFO_STATUS+3)
> arch/m68k/kernel/entry.S:60: Error: invalid instruction for this
> architecture; needs 68000 or higher (68000 [68ec000, 68hc000, 68hc001,
> 68008, 68302, 68306, 68307, 68322, 68356], 68010, 68020 [68k,
> 68ec020], 68030 [68ec030], 68040 [68ec040], 68060 [68ec060], cpu32
> [68330, 68331, 68332, 68333, 68334, 68336, 68340, 68341, 68349,
> 68360], fidoa [fido]) -- statement `andb #(~((1<<1))),%a1@(16+3)'
> ignored
>
>> rts
> Gr{oetje,eeting}s,
>
> Geert
>