Messages in this thread | | | From | Lai Jiangshan <> | Date | Thu, 7 Apr 2022 15:03:08 +0800 | Subject | Re: [PATCH V4 1/7] x86/traps: Move pt_regs only in fixup_bad_iret() |
| |
On Thu, Apr 7, 2022 at 3:00 AM Borislav Petkov <bp@alien8.de> wrote: > > On Fri, Mar 18, 2022 at 10:30:10PM +0800, Lai Jiangshan wrote: > > From: Lai Jiangshan <jiangshan.ljs@antgroup.com> > > > > fixup_bad_iret() and sync_regs() have similar arguments and do similar > > work that copies full or partial pt_regs to a place and switches stack > > after return. They are quite the same, but fixup_bad_iret() not only > > copies the pt_regs but also the return address of error_entry() while > > What return address of error_entry()? You lost me here.
"return address" is the return address of a function which is error_entry() here.
https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html
Or error_entry_ret in struct bad_iret_stack which is being removed in the change.
> > So fixup_bad_iret() gets the stack ptr passed in by doing: > > mov %rsp, %rdi > call fixup_bad_iret > mov %rax, %rsp > > > and error_regs() > > movq %rsp, %rdi /* arg0 = pt_regs pointer */ > call sync_regs > movq %rax, %rsp /* switch stack */ > > the same way.
They are not the same way.
sync_regs() is called before the return address of error_entry() popped into %r12 while fixup_bad_iret() is called with the return address of error_entry() still on the stack. And the primitives of fixup_bad_iret() and sync_regs() are different which also means they are not the same way.
After this change, they become the same way.
IMO, sync_regs() is grace while fixup_bad_iret() is a bad C function or is not a pure C function because it is handling the return address of its parent function which is better done by the compiler or ASM code.
> > Confused. > > > It is prepared for later patch to do the stack switch after the > > error_entry() which simplifies the code further. > > Looking at your next patch, is all this dance done just so that you can > do > > leaq 8(%rsp), %rdi > > in order to pass in pt_regs to both functions? > > And get rid of the saving/restoring %r12? > > Is that what the whole noise is about?
The point is to make fixup_bad_iret() a normal C function and the same as sync_regs() in calling.
The next patch makes error_entry() as a bunch of ASM code compiled from a C function and pave the road to really convert it to a C function.
Getting rid of the saving/restoring the return address in %r12 is necessary since a C function can't save/restore the return address.
Thanks Lai
> > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
I'm sorry for using top-posting and "This patch". I remember it.
| |