RE: [PATCH 05/15] x86: Implement function_nocfi
From: Rasmus Villemoes > Sent: 19 April 2021 09:40 > > On 17/04/2021 00.28, Kees Cook wrote: > > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > > >> The > >> foo symbol would point to whatever magic is needed. > > > > No, the symbol points to the jump table entry. Direct calls get minimal > > overhead and indirect calls can add the "is this function in the right > > table" checking. > > > > > > But note that this shouldn't turn into a discussion of "maybe Clang could > > do CFI differently"; this is what Clang has. > > Why not? In particular, I'd really like somebody to answer the question > "why not just store a cookie before each address-taken or > external-linkage function?". > > (1) direct calls get _no_ overhead, not just "minimal". > (2) address comparison, intra- or inter-module, Just Works, no need to > disable one sanity check to get others. > (3) The overhead and implementation of the signature check is the same > for inter- and intra- module calls. > (4) passing (unsigned long)sym into asm code or stashing it in a table > Just Works. > > How much code would be needed on the clang side to provide a > --cfi-mode=inline ? > > The only issue, AFAICT, which is also a problem being handled in the > current proposal, is indirect calls to asm code from C. They either have > to be instrumented to not do any checking (which requires callers to > know that the pointer they have might point to an asm-implementation), > or the asm code could be taught to emit those cookies as well - which > would provide even better coverage. Something like > > CFI_COOKIE(foo) > foo: > ... > > with CFI_COOKIE expanding to nothing when !CONFIG_CFI, and otherwise to > (something like) ".quad cfi_cookie__foo" ; with some appropriate Kbuild > and compiler magic to generate a table of cfi_cookie__* defines from a > header file with the prototypes. I'm wondering what 'threat models' CFI is trying to protect from. Adding code to code doing 'call indirect' can only protect against calls to 'inappropriate' functions. You may also be worried about whether functions are being called from the right place - someone might manage to 'plant' an indirect jump somewhere - probably more likely than overwriting an address. But a truly malicious caller will be able to subvert most checks. And non-malicious errors can be largely eliminated by strengthening normal compiler type checking. A simple run-time check would be adding an extra 'hidden' function parameter that is a hash of the prototype. Especially if the hash is based on a build-id - so differs between kernel builds. Having the caller scan a list of valid targets seems just broken. You might as well replace the function pointer with an index into the array of valid targets. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 05/15] x86: Implement function_nocfi
> On Apr 19, 2021, at 8:26 AM, David Laight wrote: > > From: Andy Lutomirski >> Sent: 18 April 2021 01:12 > .. >> 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. > > Maybe: > > s/opaque_symbol/entry_SYSCALL_64/ > Cute. OTOH, I’m not sure whether that has much benefit, and having a single type for all of this allows it to be declared just once. I suppose the magic could be wrapped in a macro, though. >David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales)
Re: [PATCH 05/15] x86: Implement function_nocfi
Why not? In particular, I'd really like somebody to answer the question "why not just store a cookie before each address-taken or external-linkage function?". FWIIW, this was done before (at least twice): First with grsecurity/PaX RAP (https://grsecurity.net/rap_faq) then with kCFI (https://www.blackhat.com/docs/asia-17/materials/asia-17-Moreira-Drop-The-Rop-Fine-Grained-Control-Flow-Integrity-For-The-Linux-Kernel-wp.pdf, https://github.com/kcfi/kcfi - which is no longer maintained). At the time I worked on kCFI someone raised a concern regarding this cookie-based design being mutually exclusive to execute-only memories (XOM), what, if XOM is really relevant to someone, should be a valid concern. Since design is being questioned, an x86/CET-specific third design for CFI was recently discussed here: https://www.openwall.com/lists/kernel-hardening/2021/02/11/1 -- I assume that, arch-dependency considered, this should be easier to integrate when compared to clang-cfi. Also, given that it is based on CET, this also has the benefit of constraining mispeculations (which is a nice side-effect). Tks, Joao
RE: [PATCH 05/15] x86: Implement function_nocfi
From: Andy Lutomirski > Sent: 18 April 2021 01:12 .. > 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. Maybe: s/opaque_symbol/entry_SYSCALL_64/ David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sun, Apr 18, 2021 at 3:57 PM Andy Lutomirski wrote: > > On Sun, Apr 18, 2021 at 9:17 AM Thomas Gleixner wrote: > > > > On Sat, Apr 17 2021 at 17:11, Andy Lutomirski wrote: > > > On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner > > > wrote: > > >> 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. > > > > Makes sense. > > Sami, do you want to do this as part of your series or should I write a patch? I can certainly include this in the next version, but that might have to wait a bit for compiler changes, so if you want this done sooner, please go ahead. Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17, 2021 at 3:16 AM Thomas Gleixner wrote: > > On Sat, Apr 17 2021 at 01:02, Thomas Gleixner wrote: > > On Fri, Apr 16 2021 at 15:37, Kees Cook wrote: > > > >> On Fri, Apr 16, 2021 at 03:20:17PM -0700, Andy Lutomirski wrote: > >>> But obviously there is code that needs real function pointers. How > >>> about making this a first-class feature, or at least hacking around it > >>> more cleanly. For example, what does this do: > >>> > >>> char entry_whatever[]; > >>> wrmsrl(..., (unsigned long)entry_whatever); > >> > >> This is just casting. It'll still resolve to the jump table entry. > >> > >>> or, alternatively, > >>> > >>> extern void func() __attribute__((nocfi)); > >> > >> __nocfi says func() should not perform checking of correct jump table > >> membership for indirect calls. > >> > >> But we don't want a global marking for a function to be ignored by CFI; > >> we don't want functions to escape CFI -- we want specific _users_ to > >> either not check CFI for indirect calls (__nocfi) or we want specific > >> passed addresses to avoid going through the jump table > >> (function_nocfi()). > > > > And that's why you mark entire files to be exempt without any rationale > > why it makes sense. > > The reason why you have to do that is because function_nocfi() is not > provided by the compiler. > > So you need to hack around that with that macro which fails to work > e.g. for the idt data arrays. > > Is there any fundamental reason why the compiler does not provide that > in a form which allows to use it everywhere? I'm not aware of a fundamental reason why the compiler couldn't provide a built-in here. This series attempts to work with what's available at the moment, and admittedly that's not quite ideal on x86. > It's not too much asked from a tool which provides new functionality to > provide it in a way which is usable. Sure, that's reasonable. I'll talk to our compiler folks and see how we can proceed here. Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On 17/04/2021 00.28, Kees Cook wrote: > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: >> The >> foo symbol would point to whatever magic is needed. > > No, the symbol points to the jump table entry. Direct calls get minimal > overhead and indirect calls can add the "is this function in the right > table" checking. > > > But note that this shouldn't turn into a discussion of "maybe Clang could > do CFI differently"; this is what Clang has. Why not? In particular, I'd really like somebody to answer the question "why not just store a cookie before each address-taken or external-linkage function?". (1) direct calls get _no_ overhead, not just "minimal". (2) address comparison, intra- or inter-module, Just Works, no need to disable one sanity check to get others. (3) The overhead and implementation of the signature check is the same for inter- and intra- module calls. (4) passing (unsigned long)sym into asm code or stashing it in a table Just Works. How much code would be needed on the clang side to provide a --cfi-mode=inline ? The only issue, AFAICT, which is also a problem being handled in the current proposal, is indirect calls to asm code from C. They either have to be instrumented to not do any checking (which requires callers to know that the pointer they have might point to an asm-implementation), or the asm code could be taught to emit those cookies as well - which would provide even better coverage. Something like CFI_COOKIE(foo) foo: ... with CFI_COOKIE expanding to nothing when !CONFIG_CFI, and otherwise to (something like) ".quad cfi_cookie__foo" ; with some appropriate Kbuild and compiler magic to generate a table of cfi_cookie__* defines from a header file with the prototypes. Rasmus
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sun, Apr 18, 2021 at 9:17 AM Thomas Gleixner wrote: > > On Sat, Apr 17 2021 at 17:11, Andy Lutomirski wrote: > > On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner wrote: > >> 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. > > Makes sense. Sami, do you want to do this as part of your series or should I write a patch?
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17 2021 at 17:11, Andy Lutomirski wrote: > On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner wrote: >> 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. Makes sense.
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17, 2021 at 4:53 PM Thomas Gleixner wrote: > > On Sat, Apr 17 2021 at 16:19, Andy Lutomirski wrote: > > On Fri, Apr 16, 2021 at 4:40 PM Kees Cook 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
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17 2021 at 16:19, Andy Lutomirski wrote: > On Fri, Apr 16, 2021 at 4:40 PM Kees Cook 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); and static unsigned long foo[] = { function_nocfi(bar1), function_nocfi(bar2), }; which are pretty much the only possible 2 usecases. If the compiler wants to have function_nocfi_in_code() and function_nocfi_const() because it can't figure it out on it's own, then I can live with that, but that's still several orders of magnitudes better than having to work around it by whatever nasty macro/struct magic. I don't care about the slightly more unreadable code, but if that builtin has a descriptive name, then it's even useful for documentary purposes. And it can be easily grepped for which makes it subject to static code analysers which can help to detect abuse. Anything which requires to come up with half baken constructs to work around the shortcomings of the compiler are wrong to begin with. Thanks, tglx
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 4:40 PM Kees Cook 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.
Re: [PATCH 05/15] x86: Implement function_nocfi
> On Apr 17, 2021, at 7:20 AM, David Laight wrote: > > From: Kees Cook >> Sent: 16 April 2021 23:28 >> >>> On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: >>> On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > __nocfi only disables CFI checking in a function, the compiler still > changes function addresses to point to the CFI jump table, which is > why we need function_nocfi(). So call it __func_addr() or get_function_addr() or so, so that at least it is clear what this does. >>> >>> This seems backwards to me. If I do: >>> >>> extern void foo(some signature); >>> >>> then I would, perhaps naively, expect foo to be the actual symbol that >>> gets called >> >> Yes. >> >>> and for the ABI to be changed to do the CFI checks. >> >> Uh, no? There's no ABI change -- indirect calls are changed to do the >> checking. >> >>> The >>> foo symbol would point to whatever magic is needed. >> >> No, the symbol points to the jump table entry. Direct calls get minimal >> overhead and indirect calls can add the "is this function in the right >> table" checking. > > > Isn't this a bit like one of the PPC? ABI where function addresses > aren't (always) the entry point. > IIRC is causes all sorts of obscure grief. > > I'd also like to know how indirect calls are actually expected to work. > The whole idea is that the potential targets might be in a kernel module > that is loaded at run time. > > Scanning a list of possible targets has to be a bad design decision. > > If you are trying to check that the called function has the right > prototype, stashing a hash of the prototype (or a known random number) > before the entry point would give most of the benefits without most > of the costs. > The linker could be taught to do the same test (much like name mangling > but without the crap user experience). > > That scheme only has the downside of a data cache miss and (hopefully) > statically predicted correct branch (hmm - except you don't want to > speculatively execute the wrong function) and polluting the data cache > with code. I admit I was quite surprised by the actual CFI implementation. I would have expected a CFI’d function pointer to actually point to a little descriptor that contains a hash of the function’s type. The whole bit vector thing seems quite inefficient. > > This all seems like a ploy to force people to buy faster cpus. > >David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales) >
RE: [PATCH 05/15] x86: Implement function_nocfi
From: Kees Cook > Sent: 16 April 2021 23:28 > > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > > > > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > > > __nocfi only disables CFI checking in a function, the compiler still > > > > changes function addresses to point to the CFI jump table, which is > > > > why we need function_nocfi(). > > > > > > So call it __func_addr() or get_function_addr() or so, so that at least > > > it is clear what this does. > > > > > > > This seems backwards to me. If I do: > > > > extern void foo(some signature); > > > > then I would, perhaps naively, expect foo to be the actual symbol that > > gets called > > Yes. > > > and for the ABI to be changed to do the CFI checks. > > Uh, no? There's no ABI change -- indirect calls are changed to do the > checking. > > > The > > foo symbol would point to whatever magic is needed. > > No, the symbol points to the jump table entry. Direct calls get minimal > overhead and indirect calls can add the "is this function in the right > table" checking. Isn't this a bit like one of the PPC? ABI where function addresses aren't (always) the entry point. IIRC is causes all sorts of obscure grief. I'd also like to know how indirect calls are actually expected to work. The whole idea is that the potential targets might be in a kernel module that is loaded at run time. Scanning a list of possible targets has to be a bad design decision. If you are trying to check that the called function has the right prototype, stashing a hash of the prototype (or a known random number) before the entry point would give most of the benefits without most of the costs. The linker could be taught to do the same test (much like name mangling but without the crap user experience). That scheme only has the downside of a data cache miss and (hopefully) statically predicted correct branch (hmm - except you don't want to speculatively execute the wrong function) and polluting the data cache with code. This all seems like a ploy to force people to buy faster cpus. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17 2021 at 01:02, Thomas Gleixner wrote: > On Fri, Apr 16 2021 at 15:37, Kees Cook wrote: > >> On Fri, Apr 16, 2021 at 03:20:17PM -0700, Andy Lutomirski wrote: >>> But obviously there is code that needs real function pointers. How >>> about making this a first-class feature, or at least hacking around it >>> more cleanly. For example, what does this do: >>> >>> char entry_whatever[]; >>> wrmsrl(..., (unsigned long)entry_whatever); >> >> This is just casting. It'll still resolve to the jump table entry. >> >>> or, alternatively, >>> >>> extern void func() __attribute__((nocfi)); >> >> __nocfi says func() should not perform checking of correct jump table >> membership for indirect calls. >> >> But we don't want a global marking for a function to be ignored by CFI; >> we don't want functions to escape CFI -- we want specific _users_ to >> either not check CFI for indirect calls (__nocfi) or we want specific >> passed addresses to avoid going through the jump table >> (function_nocfi()). > > And that's why you mark entire files to be exempt without any rationale > why it makes sense. The reason why you have to do that is because function_nocfi() is not provided by the compiler. So you need to hack around that with that macro which fails to work e.g. for the idt data arrays. Is there any fundamental reason why the compiler does not provide that in a form which allows to use it everywhere? It's not too much asked from a tool which provides new functionality to provide it in a way which is usable. Thanks, tglx
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 03:52:44PM -0700, Andy Lutomirski wrote: > Maybe ABI is the wrong word, or maybe I'm not fully clued in. But, if I do: > > extern void call_it(void (*ptr)(void)); > > and I define call_it in one translation unit and call it from another, > the ABI effectively changed, right? Because ptr is (depending on the > "canonical" mode) no longer a regular function pointer. Right, I was thinking maybe "calling convention", or really, "the ability to still use 'ptr' as if it were a function". Which is true, yes. It's just that 'ptr' is aimed at a jump table that jumps to the "true" function body. > 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. > 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)); Which is basically what already exists, just with a different name. > 2b. Maybe optional: convert from function pointer to bit pattern of > actual symbol. > > If someone gives me a real, correctly typed C pointer representing a > function pointer, I want a way to find the address of the body of the > function. Then we can use it for things that aren't *calling* it per > se, e.g. disassembling it. This is not necessarily a fully formed > thought right now, but I think something along these lines might be > needed. > > The reverse of 2b (converting from actual symbol to function pointer) > might be equivalent to 1, or it might not. I suppose there are some > subtleties here. > > Be that as it may, it sounds like right now clang has some issues > interoperating with asm when CFI is enabled. If so, clang needs to be > improved. > > (The unsigned long hack is not necessarily good enough. I should be able to > do: > > .global func > func: > ; C ABI compliant code here > > extern void func(void) [attribute as in 1] > > unsigned long actual_address = [something clang actually understands](func); > > If this thing refuses to work when fed a nonconstant function pointer > because of some genuinely good reason, fine. But, if 'func' is an > actual literal symbol name, this thing needs to be compile-time > constant expression. Okay, you're saying you want __builtin_gimme_body_p() to be a constant expression for the compiler, not inline asm? Given the very few places this is expected to be used, and the fact that it works as-is already, why is this additional requirement useful? > > So, instead of a cast, a wrapper is used to bypass instrumentation in > > the very few cases its needed. > > NAK. The compiler needs to cooperate IMO. It's trying very hard. ;) > > But note that this shouldn't turn into a discussion of "maybe Clang could > > do CFI differently"; this is what Clang has. > > > > https://clang.llvm.org/docs/ControlFlowIntegrity.html > > If this is what Clang has, and Clang won't improve, then we can just > not apply these patches... I'm not saying Clang can't change -- I'm saying redesigning the entire implementation of Clang's CFI isn't feasible, and I want to avoid having that become the requirement because that's unreasonable. Clang's current CFI works for many other projects,
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16 2021 at 15:37, Kees Cook wrote: > On Fri, Apr 16, 2021 at 03:20:17PM -0700, Andy Lutomirski wrote: >> But obviously there is code that needs real function pointers. How >> about making this a first-class feature, or at least hacking around it >> more cleanly. For example, what does this do: >> >> char entry_whatever[]; >> wrmsrl(..., (unsigned long)entry_whatever); > > This is just casting. It'll still resolve to the jump table entry. > >> or, alternatively, >> >> extern void func() __attribute__((nocfi)); > > __nocfi says func() should not perform checking of correct jump table > membership for indirect calls. > > But we don't want a global marking for a function to be ignored by CFI; > we don't want functions to escape CFI -- we want specific _users_ to > either not check CFI for indirect calls (__nocfi) or we want specific > passed addresses to avoid going through the jump table > (function_nocfi()). And that's why you mark entire files to be exempt without any rationale why it makes sense. Thanks, tglx
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 03:52:44PM -0700, Andy Lutomirski wrote: > > > char entry_whatever[]; > > > wrmsrl(..., (unsigned long)entry_whatever); > > > > This is just casting. It'll still resolve to the jump table entry. > > How? As far as clang is concerned, entry_whatever isn't a function at > all. What jump table entry? Whoops, sorry, I misread the [] as (). I thought you were just showing an arbitrary function declaration, but I see what you mean now. I am digesting the rest of your email now... :) -- Kees Cook
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 3:28 PM Kees Cook wrote: > > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > > > > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > > > __nocfi only disables CFI checking in a function, the compiler still > > > > changes function addresses to point to the CFI jump table, which is > > > > why we need function_nocfi(). > > > > > > So call it __func_addr() or get_function_addr() or so, so that at least > > > it is clear what this does. > > > > > > > This seems backwards to me. If I do: > > > > extern void foo(some signature); > > > > then I would, perhaps naively, expect foo to be the actual symbol that > > gets called > > Yes. > > > and for the ABI to be changed to do the CFI checks. > > Uh, no? There's no ABI change -- indirect calls are changed to do the > checking. Maybe ABI is the wrong word, or maybe I'm not fully clued in. But, if I do: extern void call_it(void (*ptr)(void)); and I define call_it in one translation unit and call it from another, the ABI effectively changed, right? Because ptr is (depending on the "canonical" mode) no longer a regular function pointer. > > char entry_whatever[]; > > wrmsrl(..., (unsigned long)entry_whatever); > > This is just casting. It'll still resolve to the jump table entry. How? As far as clang is concerned, entry_whatever isn't a function at all. What jump table entry? > > > or, alternatively, > > > > extern void func() __attribute__((nocfi)); > > __nocfi says func() should not perform checking of correct jump table > membership for indirect calls. > > But we don't want a global marking for a function to be ignored by CFI; > we don't want functions to escape CFI -- we want specific _users_ to > either not check CFI for indirect calls (__nocfi) or we want specific > passed addresses to avoid going through the jump table > (function_nocfi()). Maybe I spelled it badly, and I maybe I requested the wrong thing. Here are actual required use cases. 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. 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. 2b. Maybe optional: convert from function pointer to bit pattern of actual symbol. If someone gives me a real, correctly typed C pointer representing a function pointer, I want a way to find the address of the body of the function. Then we can use it for things that aren't *calling* it per se, e.g. disassembling it. This is not necessarily a fully formed thought right now, but I think something along these lines might be needed. The reverse of 2b (converting from actual symbol to function pointer) might be equivalent to 1, or it might not. I suppose there are some subtleties here. Be that as it may, it sounds like right now clang has some issues interoperating with asm when CFI is enabled. If so, clang needs to be improved. (The unsigned long hack is not necessarily good enough. I should be able to do: .global func func: ; C ABI compliant code here extern void func(void) [attribute as in 1] unsigned long actual_address = [something clang actually understands](func); If this thing refuses to work when fed a nonconstant function pointer because of some genuinely good reason, fine. But, if 'func' is an actual literal symbol name, this thing needs to be compile-time constant expression. > > So, instead of a cast, a wrapper is used to bypass instrumentation in > the very few cases its needed. NAK. The compiler needs to cooperate IMO. > > (Note that such a wrapper is no-op without CFI enabled.) > But note that this shouldn't turn into a discussion of "maybe Clang could > do CFI differently"; this is what Clang has. > > https://clang.llvm.org/docs/ControlFlowIntegrity.html If this is what Clang
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 03:20:17PM -0700, Andy Lutomirski wrote: > But obviously there is code that needs real function pointers. How > about making this a first-class feature, or at least hacking around it > more cleanly. For example, what does this do: > > char entry_whatever[]; > wrmsrl(..., (unsigned long)entry_whatever); This is just casting. It'll still resolve to the jump table entry. > or, alternatively, > > extern void func() __attribute__((nocfi)); __nocfi says func() should not perform checking of correct jump table membership for indirect calls. But we don't want a global marking for a function to be ignored by CFI; we don't want functions to escape CFI -- we want specific _users_ to either not check CFI for indirect calls (__nocfi) or we want specific passed addresses to avoid going through the jump table (function_nocfi()). So, instead of a cast, a wrapper is used to bypass instrumentation in the very few cases its needed. (Note that such a wrapper is no-op without CFI enabled.) -- Kees Cook
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > > __nocfi only disables CFI checking in a function, the compiler still > > > changes function addresses to point to the CFI jump table, which is > > > why we need function_nocfi(). > > > > So call it __func_addr() or get_function_addr() or so, so that at least > > it is clear what this does. > > > > This seems backwards to me. If I do: > > extern void foo(some signature); > > then I would, perhaps naively, expect foo to be the actual symbol that > gets called Yes. > and for the ABI to be changed to do the CFI checks. Uh, no? There's no ABI change -- indirect calls are changed to do the checking. > The > foo symbol would point to whatever magic is needed. No, the symbol points to the jump table entry. Direct calls get minimal overhead and indirect calls can add the "is this function in the right table" checking. > I assume I'm > missing something. Further symbol vs address stuff is discussed here: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/cfi=ff301ceb5299551c3650d0e07ba879b766da4cc0 But note that this shouldn't turn into a discussion of "maybe Clang could do CFI differently"; this is what Clang has. https://clang.llvm.org/docs/ControlFlowIntegrity.html -- Kees Cook
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 3:14 PM Borislav Petkov wrote: > > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > > > > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > > > __nocfi only disables CFI checking in a function, the compiler still > > > > changes function addresses to point to the CFI jump table, which is > > > > why we need function_nocfi(). > > > > > > So call it __func_addr() or get_function_addr() or so, so that at least > > > it is clear what this does. > > > > > > > This seems backwards to me. If I do: > > > > extern void foo(some signature); > > > > then I would, perhaps naively, expect foo to be the actual symbol that > > I'm just reading the patch: > > ... The function_nocfi macro always returns the address of the > + * actual function instead. > + */ > +#define function_nocfi(x) ({ \ > + void *addr; \ > + asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\ > + addr; > > so it does a rip-relative load into a reg which ends up with the function > address. This is horrible. We made a mistake adapting the kernel to GCC's nonsensical stack protector ABI, especially on 32-bit, instead of making GCC fix it. Let's not repeat this with clang please. Sami, I'm assuming that: extern void func(void); results in anything that takes a pointer to func getting a pointer to some special magic descriptor instead of to func, so that: void (*ptr)(void); ptr = func; ptr(); does the right thing. Then void (*)(void) is no longer a raw pointer. Fine. But obviously there is code that needs real function pointers. How about making this a first-class feature, or at least hacking around it more cleanly. For example, what does this do: char entry_whatever[]; wrmsrl(..., (unsigned long)entry_whatever); or, alternatively, extern void func() __attribute__((nocfi)); void (*ptr)(void); ptr = func; /* probably fails to compile -- invalid conversion */ (unsigned long)func /* returns the actual pointer */ func(); /* works like normal */ And maybe allow this too: void (*ptr)(void) __attribute__((nocfi); ptr = func; ptr(); /* emits an unchecked call. maybe warns, too. anyone who does this needs to be extremely careful. */ --Andy
Re: [PATCH 05/15] x86: Implement function_nocfi
On Sat, Apr 17, 2021 at 12:02:51AM +0200, Borislav Petkov wrote: > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > __nocfi only disables CFI checking in a function, the compiler still > > changes function addresses to point to the CFI jump table, which is > > why we need function_nocfi(). > > So call it __func_addr() or get_function_addr() or so, so that at least > it is clear what this does. FWIW, it's been renamed already. I'll CC Mark back into the thread. https://lore.kernel.org/lkml/20210325101655.GB36570@C02TD0UTHF1T.local/ > Also, am I going to get a steady stream of patches adding that wrapper > to function names or is this it? IOW, have you built an allyesconfig to > see how many locations need touching? Nooo. Much like __nocfi, this should be extremely rare and is only used in places that must not be doing CFI nor working on the jump tables (e.g. the syscall MSR). There list for arm64 in -next, for example, is short: 429d9a552e81 arm64: ftrace: use function_nocfi for ftrace_call fbcdf27674bc arm64: add __nocfi to __apply_alternatives f2324191e959 arm64: add __nocfi to functions that jump to a physical address c4a384170f17 arm64: use function_nocfi with __pa_symbol 5198a15901d2 psci: use function_nocfi for cpu_resume 8e284f3ebed2 bpf: disable CFI in dispatcher functions -- Kees Cook
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote: > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > > __nocfi only disables CFI checking in a function, the compiler still > > > changes function addresses to point to the CFI jump table, which is > > > why we need function_nocfi(). > > > > So call it __func_addr() or get_function_addr() or so, so that at least > > it is clear what this does. > > > > This seems backwards to me. If I do: > > extern void foo(some signature); > > then I would, perhaps naively, expect foo to be the actual symbol that I'm just reading the patch: ... The function_nocfi macro always returns the address of the + * actual function instead. + */ +#define function_nocfi(x) ({ \ + void *addr; \ + asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\ + addr; so it does a rip-relative load into a reg which ends up with the function address. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16 2021 at 14:49, Sami Tolvanen wrote: > On Fri, Apr 16, 2021 at 2:18 PM Borislav Petkov wrote: >> In file included from ./include/linux/ftrace.h:22:0, >> from ./include/linux/init_task.h:9, >> from init/init_task.c:2: >> ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: >> ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of >> function ‘function_nocfi’ [-Werror=implicit-function-declaration] > > This is defined in linux-next, but I do see another issue, which I'll > fix in v2. Note that CFI_CLANG itself cannot be selected on 32-bit > x86. Sure and because of that it's overrated to make sure that it does not break the build. I know, sekurity ... But aside of that when looking at the rest of the series, then I really have to ask whether the only way to address this is to make a large amount of code unreadable like this: - wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64); + wrmsrl(MSR_LSTAR, (unsigned long)function_nocfi(entry_SYSCALL_64)); plus a gazillion of similar changes. This is unreadable garbage. Thanks, tglx
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov wrote: > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > > __nocfi only disables CFI checking in a function, the compiler still > > changes function addresses to point to the CFI jump table, which is > > why we need function_nocfi(). > > So call it __func_addr() or get_function_addr() or so, so that at least > it is clear what this does. > This seems backwards to me. If I do: extern void foo(some signature); then I would, perhaps naively, expect foo to be the actual symbol that gets called and for the ABI to be changed to do the CFI checks. The foo symbol would point to whatever magic is needed. I assume I'm missing something.
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote: > __nocfi only disables CFI checking in a function, the compiler still > changes function addresses to point to the CFI jump table, which is > why we need function_nocfi(). So call it __func_addr() or get_function_addr() or so, so that at least it is clear what this does. Also, am I going to get a steady stream of patches adding that wrapper to function names or is this it? IOW, have you built an allyesconfig to see how many locations need touching? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 2:18 PM Borislav Petkov wrote: > > On Fri, Apr 16, 2021 at 01:38:34PM -0700, Sami Tolvanen wrote: > > With CONFIG_CFI_CLANG, the compiler replaces function addresses in > > instrumented C code with jump table addresses. This change implements > > the function_nocfi() macro, which returns the actual function address > > instead. > > > > Signed-off-by: Sami Tolvanen > > --- > > arch/x86/include/asm/page.h | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h > > index 7555b48803a8..5499a05c44fc 100644 > > --- a/arch/x86/include/asm/page.h > > +++ b/arch/x86/include/asm/page.h > > @@ -71,6 +71,20 @@ static inline void copy_user_page(void *to, void *from, > > unsigned long vaddr, > > extern bool __virt_addr_valid(unsigned long kaddr); > > #define virt_addr_valid(kaddr) __virt_addr_valid((unsigned long) > > (kaddr)) > > > > +#ifdef CONFIG_CFI_CLANG > > Almost every patch is talking about this magical config symbol but it > is nowhere to be found. How do I disable it, is there a Kconfig entry > somewhere, etc, etc? As I mentioned in the cover letter, this series is based on linux-next. I forgot to include a link to the original patch series that adds CONFIG_CFI_CLANG, but I'll be sure to point to it in the next version. Sorry about the confusion. > > +/* > > + * With CONFIG_CFI_CLANG, the compiler replaces function address > > + * references with the address of the function's CFI jump table > > + * entry. The function_nocfi macro always returns the address of the > > + * actual function instead. > > + */ > > +#define function_nocfi(x) ({ \ > > + void *addr; \ > > + asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\ > > + addr; \ > > +}) > > Also, eww. > > It seems all these newfangled things we get, mean obfuscating code even > more. What's wrong with using __nocfi instead? That's nicely out of the > way so slap that in front of functions. __nocfi only disables CFI checking in a function, the compiler still changes function addresses to point to the CFI jump table, which is why we need function_nocfi(). > Also, did you even build this on, say, 32-bit allmodconfig? > > Oh well. > > In file included from ./include/linux/ftrace.h:22:0, > from ./include/linux/init_task.h:9, > from init/init_task.c:2: > ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: > ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function > ‘function_nocfi’ [-Werror=implicit-function-declaration] This is defined in linux-next, but I do see another issue, which I'll fix in v2. Note that CFI_CLANG itself cannot be selected on 32-bit x86. Sami
Re: [PATCH 05/15] x86: Implement function_nocfi
On Fri, Apr 16, 2021 at 01:38:34PM -0700, Sami Tolvanen wrote: > With CONFIG_CFI_CLANG, the compiler replaces function addresses in > instrumented C code with jump table addresses. This change implements > the function_nocfi() macro, which returns the actual function address > instead. > > Signed-off-by: Sami Tolvanen > --- > arch/x86/include/asm/page.h | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h > index 7555b48803a8..5499a05c44fc 100644 > --- a/arch/x86/include/asm/page.h > +++ b/arch/x86/include/asm/page.h > @@ -71,6 +71,20 @@ static inline void copy_user_page(void *to, void *from, > unsigned long vaddr, > extern bool __virt_addr_valid(unsigned long kaddr); > #define virt_addr_valid(kaddr) __virt_addr_valid((unsigned long) > (kaddr)) > > +#ifdef CONFIG_CFI_CLANG Almost every patch is talking about this magical config symbol but it is nowhere to be found. How do I disable it, is there a Kconfig entry somewhere, etc, etc? > +/* > + * With CONFIG_CFI_CLANG, the compiler replaces function address > + * references with the address of the function's CFI jump table > + * entry. The function_nocfi macro always returns the address of the > + * actual function instead. > + */ > +#define function_nocfi(x) ({ \ > + void *addr; \ > + asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));\ > + addr; \ > +}) Also, eww. It seems all these newfangled things we get, mean obfuscating code even more. What's wrong with using __nocfi instead? That's nicely out of the way so slap that in front of functions. Also, did you even build this on, say, 32-bit allmodconfig? Oh well. In file included from ./include/linux/ftrace.h:22:0, from ./include/linux/init_task.h:9, from init/init_task.c:2: ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration] # define MCOUNT_ADDR ((unsigned long)(function_nocfi(__fentry__))) ^ ./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’ return ftrace_make_nop(mod, rec, MCOUNT_ADDR); ^~~ In file included from ./include/linux/ftrace.h:22:0, from ./include/linux/perf_event.h:49, from ./include/linux/trace_events.h:10, from ./include/trace/syscall.h:7, from ./include/linux/syscalls.h:85, from init/main.c:21: ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration] # define MCOUNT_ADDR ((unsigned long)(function_nocfi(__fentry__))) ^ ./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’ return ftrace_make_nop(mod, rec, MCOUNT_ADDR); ^~~ In file included from ./include/linux/ftrace.h:22:0, from ./include/linux/perf_event.h:49, from ./include/linux/trace_events.h:10, from ./include/trace/syscall.h:7, from ./include/linux/syscalls.h:85, from init/initramfs.c:10: ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration] # define MCOUNT_ADDR ((unsigned long)(function_nocfi(__fentry__))) ^ ./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’ return ftrace_make_nop(mod, rec, MCOUNT_ADDR); ^~~ In file included from ./include/linux/ftrace.h:22:0, from ./include/linux/perf_event.h:49, from arch/x86/events/core.c:15: ./include/linux/ftrace.h: In function ‘ftrace_init_nop’: ./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration] # define MCOUNT_ADDR ((unsigned long)(function_nocfi(__fentry__))) ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette