2021-03-18 17:13:00

by Sami Tolvanen

[permalink] [raw]
Subject: [PATCH v2 02/17] cfi: add __cficanonical

With CONFIG_CFI_CLANG, the compiler replaces a function address taken
in C code with the address of a local jump table entry, which passes
runtime indirect call checks. However, the compiler won't replace
addresses taken in assembly code, which will result in a CFI failure
if we later jump to such an address in instrumented C code. The code
generated for the non-canonical jump table looks this:

<noncanonical.cfi_jt>: /* In C, &noncanonical points here */
jmp noncanonical
...
<noncanonical>: /* function body */
...

This change adds the __cficanonical attribute, which tells the
compiler to use a canonical jump table for the function instead. This
means the compiler will rename the actual function to <function>.cfi
and points the original symbol to the jump table entry instead:

<canonical>: /* jump table entry */
jmp canonical.cfi
...
<canonical.cfi>: /* function body */
...

As a result, the address taken in assembly, or other non-instrumented
code always points to the jump table and therefore, can be used for
indirect calls in instrumented code without tripping CFI checks.

Signed-off-by: Sami Tolvanen <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]> # pci.h
---
include/linux/compiler-clang.h | 1 +
include/linux/compiler_types.h | 4 ++++
include/linux/init.h | 4 ++--
include/linux/pci.h | 4 ++--
4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 6de9d0c9377e..adbe76b203e2 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -63,3 +63,4 @@
#endif

#define __nocfi __attribute__((__no_sanitize__("cfi")))
+#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 796935a37e37..d29bda7f6ebd 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -246,6 +246,10 @@ struct ftrace_likely_data {
# define __nocfi
#endif

+#ifndef __cficanonical
+# define __cficanonical
+#endif
+
#ifndef asm_volatile_goto
#define asm_volatile_goto(x...) asm goto(x)
#endif
diff --git a/include/linux/init.h b/include/linux/init.h
index b3ea15348fbd..045ad1650ed1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -220,8 +220,8 @@ extern bool initcall_debug;
__initcall_name(initstub, __iid, id)

#define __define_initcall_stub(__stub, fn) \
- int __init __stub(void); \
- int __init __stub(void) \
+ int __init __cficanonical __stub(void); \
+ int __init __cficanonical __stub(void) \
{ \
return fn(); \
} \
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 86c799c97b77..39684b72db91 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1944,8 +1944,8 @@ enum pci_fixup_pass {
#ifdef CONFIG_LTO_CLANG
#define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
class_shift, hook, stub) \
- void stub(struct pci_dev *dev); \
- void stub(struct pci_dev *dev) \
+ void __cficanonical stub(struct pci_dev *dev); \
+ void __cficanonical stub(struct pci_dev *dev) \
{ \
hook(dev); \
} \
--
2.31.0.291.g576ba9dcdaf-goog


2021-03-18 17:53:47

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v2 02/17] cfi: add __cficanonical

On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen <[email protected]> wrote:
>
> With CONFIG_CFI_CLANG, the compiler replaces a function address taken
> in C code with the address of a local jump table entry, which passes
> runtime indirect call checks. However, the compiler won't replace
> addresses taken in assembly code, which will result in a CFI failure
> if we later jump to such an address in instrumented C code. The code
> generated for the non-canonical jump table looks this:
>
> <noncanonical.cfi_jt>: /* In C, &noncanonical points here */
> jmp noncanonical
> ...
> <noncanonical>: /* function body */
> ...
>
> This change adds the __cficanonical attribute, which tells the
> compiler to use a canonical jump table for the function instead. This
> means the compiler will rename the actual function to <function>.cfi
> and points the original symbol to the jump table entry instead:
>
> <canonical>: /* jump table entry */
> jmp canonical.cfi
> ...
> <canonical.cfi>: /* function body */
> ...
>
> As a result, the address taken in assembly, or other non-instrumented
> code always points to the jump table and therefore, can be used for
> indirect calls in instrumented code without tripping CFI checks.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]> # pci.h

Irrelevant to this series, but I checked when the FN attr was first
available in clang; clang-10. (That's the minimum supported version of
clang for the kernel, and this series depends on LTO which depends on
clang-12, so no additional guards are necessary).

Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> include/linux/compiler-clang.h | 1 +
> include/linux/compiler_types.h | 4 ++++
> include/linux/init.h | 4 ++--
> include/linux/pci.h | 4 ++--
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 6de9d0c9377e..adbe76b203e2 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -63,3 +63,4 @@
> #endif
>
> #define __nocfi __attribute__((__no_sanitize__("cfi")))
> +#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 796935a37e37..d29bda7f6ebd 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -246,6 +246,10 @@ struct ftrace_likely_data {
> # define __nocfi
> #endif
>
> +#ifndef __cficanonical
> +# define __cficanonical
> +#endif
> +
> #ifndef asm_volatile_goto
> #define asm_volatile_goto(x...) asm goto(x)
> #endif
> diff --git a/include/linux/init.h b/include/linux/init.h
> index b3ea15348fbd..045ad1650ed1 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -220,8 +220,8 @@ extern bool initcall_debug;
> __initcall_name(initstub, __iid, id)
>
> #define __define_initcall_stub(__stub, fn) \
> - int __init __stub(void); \
> - int __init __stub(void) \
> + int __init __cficanonical __stub(void); \
> + int __init __cficanonical __stub(void) \
> { \
> return fn(); \
> } \
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 86c799c97b77..39684b72db91 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1944,8 +1944,8 @@ enum pci_fixup_pass {
> #ifdef CONFIG_LTO_CLANG
> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
> class_shift, hook, stub) \
> - void stub(struct pci_dev *dev); \
> - void stub(struct pci_dev *dev) \
> + void __cficanonical stub(struct pci_dev *dev); \
> + void __cficanonical stub(struct pci_dev *dev) \
> { \
> hook(dev); \
> } \
> --
> 2.31.0.291.g576ba9dcdaf-goog
>


--
Thanks,
~Nick Desaulniers