Duh.
I woke up this morning, realizing what was wrong with my patch.
On Mon, May 6, 2019 at 8:28 PM Linus Torvalds torvalds@linux-foundation.org wrote:
Yes. But I was looking at the ftrace parts because I didn't see the bug in the low-level x86 side, so...
There was nothing wrong in the *low-level* parts. There was one thing wrong with the "in3_emulate_push()" code:
memmove(new, regs, SAVED_KERNEL_REGS_SIZE);
which ends up calling an out-of-line function. One that is traced. One that will recursively result in 'int3'. Which will fill up the stack until you get a triple fault and reboot.
Stupid stupid.
Anyway, changing that to just copying things one word at a time makes everything work. The attached patch boots with the whole ftrace test thing.The only change is literally changing that memmove() into
/* Inlined "memmove()" of the pt_regs */ unsigned long *p = (unsigned long *) new; int i = SAVED_KERNEL_REGS_SIZE / sizeof(unsigned long); do { *p = p[1]; p++; } while (--i);
which I'm not entirely proud of, but it sure is still simple.
And honestly, I absolutely despise PeterZ's patch. The notion that we should suddenly say that "oh, the i386 kernel stack is odd" after 28 years of having that standard i386 stack is just crazy. And this:
arch/x86/entry/entry_32.S | 136 ++++++++++++++++++++++++++++------- ... 12 files changed, 323 insertions(+), 140 deletions(-)
vs this:
arch/x86/entry/entry_32.S | 7 +++- ... 6 files changed, 120 insertions(+), 13 deletions(-)
is still pretty damn conclusive. Not to mention that the simple approach had a truly mindbogglingly simple solution with no actual subtle changes anywhere else.
So I still claim that we should do my patch. Because it is SIMPLE. It's straightforward, and I can explain every single line in it. Even if I spent *way* too long until I realized that the "trivial" memmove() wasn't so trivial.
Linus