Re: [PATCH V4 1/7] x86/traps: Move pt_regs only in fixup_bad_iret()

From: Borislav Petkov <bp@alien8.de>
To: Lai Jiangshan <jiangshanlai@gmail.com>,
	Andy Lutomirski <luto@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>, X86 ML <x86@kernel.org>,
	Lai Jiangshan <jiangshan.ljs@antgroup.com>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Thomas Tai <thomas.tai@oracle.com>,
	"Chang S. Bae" <chang.seok.bae@intel.com>,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH V4 1/7] x86/traps: Move pt_regs only in fixup_bad_iret()
Date: Thu, 7 Apr 2022 10:22:25 +0200	[thread overview]
Message-ID: <Yk6fQfgo975pW3t0@zn.tnic> (raw)
In-Reply-To: <CAJhGHyBFbtyUs-nf0+gWm2a3hiS5BxZ3jk=sbGNw-4ShB8AtzQ@mail.gmail.com>

On Thu, Apr 07, 2022 at 03:03:08PM +0800, Lai Jiangshan wrote:
> 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.

Maybe there was a reason it was done this way:

  b645af2d5905 ("x86_64, traps: Rework bad_iret")

although I don't see anything relevant in the text explaining this.

Andy?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette