Date:   Fri, 10 Mar 2023 17:20:04 -0800
From:   Josh Poimboeuf <jpoimboe@kernel.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        Mark Rutland <mark.rutland@arm.com>,
        Jason Baron <jbaron@akamai.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>
Subject: Re: [RFC][PATCH 1/5] static_call: Make NULL static calls consistent
Message-ID: <20230311012004.vyc4kdlqjsv4zliw@treble>
References: <cover.1678474914.git.jpoimboe@kernel.org>
 <016c1e9cbdf726a885a406ff6baed85087ad1213.1678474914.git.jpoimboe@kernel.org>
 <20230310205926.GB1605437@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230310205926.GB1605437@hirez.programming.kicks-ass.net>
Precedence: bulk

On Fri, Mar 10, 2023 at 09:59:26PM +0100, Peter Zijlstra wrote:
> > -#define __static_call_cond(name)					\
> > -({									\
> > -	void *func = READ_ONCE(STATIC_CALL_KEY(name).func);		\
> > -	if (!func)							\
> > -		func = &__static_call_nop;				\
> > -	(typeof(STATIC_CALL_TRAMP(name))*)func;				\
> > -})
> 
> So a sufficiently clever compiler can optimize the above to avoid the
> actual indirect call (and resulting CFI violation, see below), because
> __static_call_nop() is inline and hence visible as an empty stub
> function. Currently none of the compilers are that clever :/

I won't hold my breath waiting for theoretical optimizations.

> This will break ARM64 I think, they don't HAVE_STATIC_CALL but do have
> CLANG_CFI, which means the above will end up being a runtime indirect
> call to a non-matching signature function.
> 
> Now, I suppose we don't actually have this happen in current code by the
> simple expedient of not actually having any static_call_cond() usage
> outside of arch code.
> 
> (/me git-grep's some and *arrrggh* trusted-keys)
> 
> I really don't think we can do this though, must not promote CFI
> violations.

Ouch, so static_call_cond() and __static_call_return0() are broken today
on CFI_CLANG + arm64.

Some ideas:

  1) Implement HAVE_STATIC_CALL for arm64.  IIRC, this wasn't worth the
     effort due to restricted branch ranges and CFI fun.

  2) Create yet another "tier" of static call implementations, for
     arches which can have the unfortunate combo of CFI_CLANG +
     !HAVE_STATIC_CALL.  CONFIG_ALMOST_DONT_HAVE_STATIC_CALL?

     The arch can define ARCH_DEFINE_STATIC_CALL_NOP() which uses inline
     asm to create a CFI-compliant NOP/BUG/whatever version of the
     function (insert lots of hand-waving).  Is the kcfi hash available
     to inline asm at build time?

  3) Use a jump label to bypass the static call instead of calling
     __static_call_nop().  NOTE: I couldn't figure out how to do this
     without angering the compiler, unless we want to change
     static_call() back to the old-school interface:

        static_call(foo, args...)

Is it Friday yet?

-- 
Josh