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;
User-Agent: Cyrus-JMAP/3.5.0-alpha0-1345-g8441cd7852-fm-20211006.001-g8441cd78
Mime-Version: 1.0
Message-Id: <ecbe49c8-b30b-456c-931a-cc65647b0958@www.fastmail.com>
In-Reply-To: <CABCJKuc+mN4vw_sanZQKcb1=SyfT4h3JK2wpBuaB2qZH3Croxg@mail.gmail.com>
References: <20211013181658.1020262-1-samitolvanen@google.com>
 <20211013181658.1020262-4-samitolvanen@google.com>
 <7377e6b9-7130-4c20-a0c8-16de4620c995@www.fastmail.com> <8735p25llh.ffs@tglx>
 <87zgra41dh.ffs@tglx>
 <CABCJKuc+mN4vw_sanZQKcb1=SyfT4h3JK2wpBuaB2qZH3Croxg@mail.gmail.com>
Date:   Fri, 15 Oct 2021 12:35:37 -0700
From:   "Andy Lutomirski" <luto@kernel.org>
To:     "Sami Tolvanen" <samitolvanen@google.com>,
        "Thomas Gleixner" <tglx@linutronix.de>
Cc:     "the arch/x86 maintainers" <x86@kernel.org>,
        "Kees Cook" <keescook@chromium.org>,
        "Josh Poimboeuf" <jpoimboe@redhat.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        "Nathan Chancellor" <nathan@kernel.org>,
        "Nick Desaulniers" <ndesaulniers@google.com>,
        "Sedat Dilek" <sedat.dilek@gmail.com>,
        "Steven Rostedt" <rostedt@goodmis.org>,
        linux-hardening@vger.kernel.org,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        llvm@lists.linux.dev
Subject: Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C
Content-Type: text/plain
Precedence: bulk

On Fri, Oct 15, 2021, at 11:42 AM, Sami Tolvanen wrote:
>>   https://lore.kernel.org/lkml/alpine.LFD.2.00.1001251002430.3574@localhost.localdomain/
>>
>> That said, I still want to have a coherent technical explanation why the
>> compiler people cannot come up with a sensible annotation for these
>> things.
>
> I can only assume they didn't think about this specific use case.

I must be missing something here.  Linux is full of C-ABI functions implemented in asm.  Just off of a quick grep:

asm_load_gs_index, memset, memmove, basically everything in arch/x86/lib/*.S

If they're just declared and called directly from C, it should just work.  But an *indirect* call needs some sort of special handling.  How does this work in your patchset?

Then we get to these nasty cases where, for some reason, we need to explicitly grab the actual entry point or we need to grab the actual literal address that we can call indirectly.  This might be alternative_call, where we're trying to be fast and we want to bypass the CFI magic because, despite what the compiler might try to infer, we are doing a direct call (so it can't be the wrong address due a runtime attack, ENDBR isn't needed, etc).  And I can easily believe that the opposite comes to mind.  And there are things like exception entries, where C calls make no sense, CFI makes no sense, and they should be totally opaque.

So I tend to think that tglx is right *and* we need an attribute, because there really are multiple things going on here.

SYM_FUNC_START(c_callable_func)
  ...
  ret
SYM_FUNC_END

extern __magic int c_callable_func(whatever);

Surely *something* needs to go where __magic is to tell the compiler that we have a function that wasn't generated by a CFI-aware compiler and that it's just a C ABI function.  (Or maybe this is completely implicit?  I can't keep track of exactly which thing generates which code in clang CFI.)

But we *also* have the read-the-address thing:

void something(void)
{
  /* actual C body */
}
alternative_call(something, someotherthing, ...);

That wants to expand to assembly code that does:

CALL [target]

where [target] is the actual first instruction of real code and not a CFI prologue.  Or, inversely, we want:

void (*ptr)(void) = something;

which (I presume -- correct me if I'm wrong) wants the CFI landing pad.  It's not the same address.

And this all wants to work both for asm-defined functions and C-defined functions.  This really is orthogonal to the is-it-asm-or-is-it-C things.  All four combinations are possible.

Does this make any sense?  I kind of thing we want the attributes and the builtin, along the lines of:

asm("call %m", function_nocfi_address(something));

or however else we wire it up.

(And, of course, the things that aren't C functions at all, like exception entries, should be opaque.)