Messages in this thread | | | From | Denys Vlasenko <> | Date | Thu, 26 Feb 2015 10:55:16 +0100 | Subject | Re: [PATCH 2/3 v3] x86: entry_64.S: always allocate complete "struct pt_regs" |
| |
On Wed, Feb 25, 2015 at 10:59 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On Wed, Feb 25, 2015 at 1:28 PM, Denys Vlasenko <dvlasenk@redhat.com> wrote: >> On 02/25/2015 09:10 PM, Andy Lutomirski wrote: >> This part? >> >> .macro FORK_LIKE func >> ENTRY(stub_\func) >> CFI_STARTPROC >> - popq %r11 /* save return address */ >> - PARTIAL_FRAME 0 >> - SAVE_REST >> - pushq %r11 /* put it back on stack */ >> + DEFAULT_FRAME 0, 8 /* offset 8: return address */ >> + SAVE_EXTRA_REGS 8 >> FIXUP_TOP_OF_STACK %r11, 8 >> - DEFAULT_FRAME 0 8 /* offset 8: return address */ >> call sys_\func >> RESTORE_TOP_OF_STACK %r11, 8 >> - ret $REST_SKIP /* pop extended registers */ >> + ret >> CFI_ENDPROC >> END(stub_\func) >> .endm >> >> FORK_LIKE clone >> FORK_LIKE fork >> FORK_LIKE vfork >> >> But the old code (SAVE_REST thing) was also saving registers here. >> It had to jump through hoops (pop return address, SAVE_REST, >> push return address) to do that. >> After the patch, "SAVE_EXTRA_REGS 8" does the same, just without >> pop/push pair. >> >> I just don't see what's wrong with it. Can you elaborate? > > SAVE_REST pushed the regs onto the stack, whereas SAVE_EXTRA_REGS just > writes them in place. It's possible for this to be called when the > regs have already been saved.
If that would be the case - that is, if SAVE_REST was saving extra copy of registers on stack, then FIXUP_TOP_OF_STACK %r11, 8 would be working on wrong locations. The "8" there says "we have full pt_regs on stack, plus extra 8 bytes (the return address)". Your conjecture would mean that in fact there would be more bytes on stack, and FIXUP_TOP_OF_STACK would corrupt iret stack. Evidently, since old code was not crashing, this wasn't happening. SAVE_REST was really creating the "tail" of pt_regs.
In addition to my previous tests, I ran my home machine with patched kernel. Unfortunately, it works for me :(
Will try on yet another machine.
-- vda
| |