linux-kernel - Re: [PATCH V4 1/7] x86/traps: Move pt_regs only in fixup_bad

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yk6fQfgo975pW3t0@zn.tnic>
Date:   Thu, 7 Apr 2022 10:22:25 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Lai Jiangshan <jiangshanlai@...il.com>,
        Andy Lutomirski <luto@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>, X86 ML <x86@...nel.org>,
        Lai Jiangshan <jiangshan.ljs@...group.com>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Tai <thomas.tai@...cle.com>,
        "Chang S. Bae" <chang.seok.bae@...el.com>,
        Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH V4 1/7] x86/traps: Move pt_regs only in fixup_bad_iret()

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