linux-kernel - Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABCJKucuun-n8w3=U6i43kVGkCgYL7kmz5wx8nYxsxUOCfzBFw@mail.gmail.com>
Date:   Fri, 15 Oct 2021 13:37:02 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-hardening@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev
Subject: Re: [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C

On Fri, Oct 15, 2021 at 12:36 PM Andy Lutomirski <luto@...nel.org> wrote:
>
> 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?

Making indirect calls to functions implemented in assembly doesn't
require special handling. The type is inferred from the C function
declaration.

> 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.

Correct, this is the main issue we're trying to solve. For low-level
entry points, we want the actual symbol address instead of the CFI
magic, both because of performance and because CFI jump tables may not
be mapped into memory with KPTI. Using an opaque type cleanly
accomplishes the goal for symbols that are not callable from C code.

> 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.)

This is implicit. Nothing is needed for this case.

> 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.

Yes, here we would ideally want to avoid the CFI stub for better
performance, but nothing actually breaks even if we don't.

> 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.

Correct.

> 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.)

I agree, there are cases where having a function attribute and/or a
built-in to stop the compiler from interfering would be useful. I'll
dust off my patch series and see how the LLVM folks feel about it.

Sami