Messages in this thread | | | From | Andy Lutomirski <> | Date | Sat, 17 Apr 2021 17:11:32 -0700 | Subject | Re: [PATCH 05/15] x86: Implement function_nocfi |
| |
On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Sat, Apr 17 2021 at 16:19, Andy Lutomirski wrote: > > On Fri, Apr 16, 2021 at 4:40 PM Kees Cook <keescook@chromium.org> wrote: > >> 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. > > The right way to solve this is that the compiler provides a builtin > > function_nocfi() +/- the naming bikeshed > > which works for > > foo = function_nocfi(bar);
I agree in general. But right now, we have, in asm/proto.h:
void entry_SYSCALL_64(void);
and that's pure nonsense. Depending on your point of view, entry_SYSCALL_64 is a symbol that resolves to an integer or it's an array of bytes containing instructions, but it is most definitely not a function void (void). So, regardless of any CFI stuff, I propose that we standardize our handling of prototypes of symbols that are opaque to the C compiler. Here are a couple of choices:
Easy one:
extern u8 entry_SYSCALL_64[];
Slightly more complicated:
struct opaque_symbol; extern struct opaque_symbol entry_SYSCALL_64;
The opaque_symbol variant avoids any possible confusion over the weird status of arrays in C, and it's hard to misuse, since struct opaque_symbol is an incomplete type.
--Andy
| |