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

[Josh Poimboeuf]

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