linux-kernel - Re: [PATCH V4 2/7] x86/entry: Switch the stack after error

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 11 Apr 2022 19:48:48 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        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>
Subject: Re: [PATCH V4 2/7] x86/entry: Switch the stack after error_entry() returns

On Mon, Apr 11, 2022 at 5:35 PM Borislav Petkov <bp@...en8.de> wrote:
>
> On Fri, Mar 18, 2022 at 10:30:11PM +0800, Lai Jiangshan wrote:
> > From: Lai Jiangshan <jiangshan.ljs@...group.com>

>
> > Switching to the stack after error_entry() makes the code simpler and
> > intuitive.
> >
> > The behavior/logic is unchanged:
> >   1) (opt) feed fixup_bad_iret() with the pt_regs pushed by ASM code
>
> opt?

I meant it as optional.

I will rewrite it as

1) feed fixup_bad_iret() with the pt_regs pushed by the ASM code if it
is a fault
caused by bad IRET.

>
> >   2) (opt) fixup_bad_iret() moves the partial pt_regs up
> >   3) feed sync_regs() with the pt_regs pushed by ASM code or returned
> >      by fixup_bad_iret()
> >   4) sync_regs() copies the whole pt_regs to kernel stack if needed
> >   5) after error_entry() and switching %rsp, it is in kernel stack with
> >      the pt_regs


>
> > After this change, error_entry() will not do fancy things with the stack
> > except when in the prolog which will be fixed in the next patch ("move
> > PUSH_AND_CLEAR_REGS out of error_entry").  This patch and the next patch
>
> "This patch" is tautology, as already said.
>
> There's no "next patch" in git.
>
> > can't be swapped because the next patch relies on this patch's stopping
> > fiddling with the return-address of error_entry(), otherwise the objtool
> > would complain.
>
> If that is the case, then those two should me merged into one!

This patch moves the epilog (switching stack) of error_entry() out of
error_entry().  The next patch moves the prolog (pushing pt_regs) out
of error_entry().  They can be separated patches.

I don't think anything wrong if the order of these two patches
is swapped.  Peter Z asked info about the ordering of other patches
and I tried moving the next patch up and saw the complaint from
the objtool.

I wanted to explain the ordering of the patches.  This explanation
should be put in the cover letter instead of in the commit message.

Thanks
Lai