Re: [PATCH v2] m68k: save extra registers on more syscall entry points

[Linus Torvalds <torvalds@linux-foundation.org>]

On Thu, Jun 17, 2021 at 6:27 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>
> I'd need specific test cases to exercise io_uring_setup in
> particular, to see whether stack offsets for pt_regs and the
> switch stack have been messed up.

I don't think doing this for io_uring_setup() will help any - the
problem is not in that system call thread itself, it's purely in the
kernel thread that it then starts.

And the fact that io_uring_setup() has the full stack frame won't then
help that kernel thread, for exactly the same reason that was true on
alpha: copy_thread() will actually _create_ the full stack, but when
we switch to it (through switch_to() -> resume()), the resume code in
arch/m68k/kernel/entry.S will switch to that stack, and then do
RESTORE_SWITCH_STACK which will consume it again.

So I think m68k should do the same thing as Eric's patch for alpha: do
the full stack for exit and exit_group, and for kernel thread creation
- or at least PF_IO_WORKER), do an extra stack frame on the kernel
stack, so that even after resume() we'll still have another copy of
the frame.

The alternative would be to do what x86 does: see __switch_to_asm().
Instead of doing that normal kernel entry/exit stack (with
SAVE_SWITCH_STACK and RESTORE_SWITCH_STACK), x86 has it's own very
special "only for task switching" stack frame thing, and leaves the
pt_regs etc entirely alone.

Of course, that "only for task switching" is _kind_of_ what the whole
SAVE_SWITCH_STACK is for - it's part of the name after all - but the
difference is that on alpha and m68k, it's also (and primarily) the
"full state" stack frame, used not just for task switching, but for
signal handling state and for ptrace too.

So in theory, it would be good to split this up:

 (a) have the signal handling and ptrace stack be one thing (maybe
rename the "SWITCH" part of the operations to something else, like
"EXTRA" or "SIGNAL" or whatever)

 (b) make a separate "for task switching only" stack frame, which is
used by that switch_to() -> resume() sequence, and that copy_thread()
has a "struct inactive_task_frame" thing for..

That way, the pt_regs/extra_regs stack frame that copy_thread()
creates wouldn't then be eaten up by the task switch.

But while that sounds like the right thing to do, it would be a rather
bigger change. I'm not entirely sure it's worth it.

Eric, comments?

                Linus