Re: [PATCH 05/15] x86: Implement function_nocfi

[Andy Lutomirski <luto@kernel.org>]

On Fri, Apr 16, 2021 at 4:40 PM Kees Cook <keescook@chromium.org> wrote:
>

> > 1. I defined a function in asm.  I want to tell clang that this
> > function is defined in asm, and for clang to behave accordingly:
> >
> > .globl func
> > func:
> >  ; do stuff
> >
> > later:
> >
> > extern void func(void) [something here];
> >
> > There really should be a way to write this correctly such that it
> > works regardless of the setting of
> > -fsanitize-cfi-canonical-jump-tables.  This should not bypass CFI.  It
> > should *work*, with CFI enforced.  If I read all the various things
> > you linked correctly, this would be something like __cfi_noncanonical,
> > and I reserve the right to think that this is a horrible name.
>
> Yes, I find the name confusing too. Without noncanonical, we'd need
> C call wrappers for every single .S function that had its address
> taken. This is very common in crypto, for example. That level of extra
> code seemed like a total non-starter. Instead, we just get a few places
> we have to mark.

The patch you linked doesn't have a noncanonical attribute, though.
So I'm not sure how to reliably call into asm from C.

(The more I think about it, the less I like "canonical".  What is
"canonical"?  The symbol?  The function body?  Something else?)

>
> > 2. I need a raw function pointer, thank you very much.  I would like
> > to be honest about it, and I don't really want to bypass CFI, but I
> > need the actual bits in the actual symbol.
> >
> > translation unit 1 defines func.  Maybe it's C with
> > -fsanitize-cfi-canonical-jump-tables, maybe it's C with
> > -fno-sanitize-cfi-canonical-jump-tables or however it's spelled, and
> > maybe it's plain asm.  Now translation unit 2 does:
> >
> > 2a. Uses a literal symbol, because it's going to modify function text
> > or poke an MSR or whatever:
> >
> > wrmsrl(MSR_WHATEVER, func);
> >
> > clang needs to give us *some* way to have a correct declaration of
> > func such that we can, without resorting to inline asm kludges, get
> > the actual bit pattern of the actual symbol.
>
> We don't want version of a global symbol alias of func that points to
> the function body, though; it's only very specific cases where this
> should be stripped (MSR, ftrace, etc).
>
> So, if there were some Clang-specific syntax for this, it would still be
> used on a case-by-case basis. It would still be something like:
>
> wrmsrl(MSR_WAT, __builtin_gimme_body_p(func));
>

> Okay, you're saying you want __builtin_gimme_body_p() to be a constant
> expression for the compiler, not inline asm?

Yes.

I admit that, in the trivial case where the asm code is *not* a
C-ABI-compliant function, giving a type that doesn't fool the compiler
into thinking that it might be is probably the best fix.  Maybe we
should standardize something, e.g.:

struct raw_symbol;  /* not defined anywhere */
#define DECLARE_RAW_SYMBOL(x) struct raw_symbol x[]

and then we write this:

DECLARE_RAW_SYMBOL(entry_SYSCALL_64);

wrmsrl(..., (unsigned long)entry_SYSCALL_64);

It would be a bit nifty if we didn't need a forward declaration, but
I'm not immediately seeing a way to do this without hacks that we'll
probably regret;

But this doesn't help the case in which the symbol is an actual
C-callable function and we want to be able to call it, too.