Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 18 Apr 2020 12:18:10 -0500
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     Andy Lutomirski <luto@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Alexandre Chartre <alexandre.chartre@oracle.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Miroslav Benes <mbenes@suse.cz>, jthierry@redhat.com
Subject: Re: [PATCH v5 02/17] objtool: Better handle IRET
Message-ID: <20200418171810.n4vcaihdfksltuqg@treble>
References: <20200416114706.625340212@infradead.org>
 <20200416115118.631224674@infradead.org>
 <8692ee18-e553-6f90-5291-62d6d57407cd@oracle.com>
 <20200417182339.GJ20730@hirez.programming.kicks-ass.net>
 <CALCETrU58muwSVDstRJQGEn20aOOHFctzZkxHRGh-yEGR9x1VA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CALCETrU58muwSVDstRJQGEn20aOOHFctzZkxHRGh-yEGR9x1VA@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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