linux-kernel - Re: [PATCH v5 02/17] objtool: Better handle IRET

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200418171810.n4vcaihdfksltuqg@treble>
Date:   Sat, 18 Apr 2020 12:18:10 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Miroslav Benes <mbenes@...e.cz>, jthierry@...hat.com
Subject: Re: [PATCH v5 02/17] objtool: Better handle IRET

On Fri, Apr 17, 2020 at 04:53:31PM -0700, Andy Lutomirski wrote:
> I'm wondering if this would be easier if we just moved the guts of
> sync_core() into asm.
> 
> In the near future, I think we want to rework it a tiny bit.  In
> particular, I think we're going to want to make sync_core() do:
> 
> if (static_cpu_has(X86_FEATURE_SERIALIZE))
>   asm volatile ("serialize");
> else
>   iret_to_self();
> 
> where iret_to_self() is the meat of the IRET hack.  And then we're
> going to add a new thingy unmask_nmi() that does iret_to_self() on
> everything except SEV-ES.  The near-term motivation is that I think we
> have some genuine bugs in a couple of corner cases:
> 
> 1. On AMD chips, if NMI hits user code with invalid CS or SS, we will
> enter on the NMI stack, switch to the normal stack, and return with
> IRET, and the IRET will fail.  And then we end up in a nasty state in
> which NMIs are masked but the code path we run doesn't expect that.
> So we should unmask_nmi() in fixup_bad_iret() or similar.  Intel CPUs
> are unaffected because Intel is differently quirky.
> 
> 2.  do_nmi() does this:
> 
>     if (user_mode(regs))
>                 mds_user_clear_cpu_buffers();
> 
> because it can't safely call prepare_exit_to_usermode().  This is a
> gross wart and I'd like to fix it.  Fixing it involves teaching the
> relevant code paths to unmask_nmis() if they're going to so IRQs-on
> exit work.
> 
> None of this is really relevant to the current patch, but it wouldn't
> be unreasonable to turn the IRET thing from an inline asm into a real
> asm function if it makes objtool's life easier.

I don't think that would make objtool's life any easier -- it still has
to understand the stack impact of the IRET-to-self thing regardless.

-- 
Josh