With CONFIG_CFI_CLANG, assembly functions called indirectly
from C code must be annotated with type identifiers to pass CFI
checking. The compiler emits a __kcfi_typeid_<function> symbol for
each address-taken function declaration in C, which contains the
expected type identifier. Add typed versions of SYM_FUNC_START and
SYM_FUNC_START_ALIAS, which emit the type identifier before the
function.
Signed-off-by: Sami Tolvanen <[email protected]>
---
include/linux/cfi_types.h | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 include/linux/cfi_types.h
diff --git a/include/linux/cfi_types.h b/include/linux/cfi_types.h
new file mode 100644
index 000000000000..dd16e755a197
--- /dev/null
+++ b/include/linux/cfi_types.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Clang Control Flow Integrity (CFI) type definitions.
+ */
+#ifndef _LINUX_CFI_TYPES_H
+#define _LINUX_CFI_TYPES_H
+
+#ifdef CONFIG_CFI_CLANG
+#include <linux/linkage.h>
+
+#ifdef __ASSEMBLY__
+/*
+ * Use the __kcfi_typeid_<function> type identifier symbol to
+ * annotate indirectly called assembly functions. The compiler emits
+ * these symbols for all address-taken function declarations in C
+ * code.
+ */
+#ifndef __CFI_TYPE
+#define __CFI_TYPE(name) \
+ .4byte __kcfi_typeid_##name
+#endif
+
+#define SYM_TYPED_ENTRY(name, fname, linkage, align...) \
+ linkage(name) ASM_NL \
+ align ASM_NL \
+ __CFI_TYPE(fname) ASM_NL \
+ name:
+
+#define __SYM_TYPED_FUNC_START_ALIAS(name, fname) \
+ SYM_TYPED_ENTRY(name, fname, SYM_L_GLOBAL, SYM_A_ALIGN)
+
+#define __SYM_TYPED_FUNC_START(name, fname) \
+ SYM_TYPED_ENTRY(name, fname, SYM_L_GLOBAL, SYM_A_ALIGN)
+
+#endif /* __ASSEMBLY__ */
+
+#else /* CONFIG_CFI_CLANG */
+
+#ifdef __ASSEMBLY__
+#define __SYM_TYPED_FUNC_START_ALIAS(name, fname) \
+ SYM_FUNC_START_ALIAS(name)
+
+#define __SYM_TYPED_FUNC_START(name, fname) \
+ SYM_FUNC_START(name)
+#endif /* __ASSEMBLY__ */
+
+#endif /* CONFIG_CFI_CLANG */
+
+#ifdef __ASSEMBLY__
+#define SYM_TYPED_FUNC_START_ALIAS(name) \
+ __SYM_TYPED_FUNC_START_ALIAS(name, name)
+
+#define SYM_TYPED_FUNC_START(name) \
+ __SYM_TYPED_FUNC_START(name, name)
+#endif /* __ASSEMBLY__ */
+
+#endif /* _LINUX_CFI_TYPES_H */
--
2.36.0.550.gb090851708-goog
On Fri, May 13, 2022 at 01:21:45PM -0700, Sami Tolvanen wrote:
> With CONFIG_CFI_CLANG, assembly functions called indirectly
> from C code must be annotated with type identifiers to pass CFI
> checking. The compiler emits a __kcfi_typeid_<function> symbol for
> each address-taken function declaration in C, which contains the
> expected type identifier. Add typed versions of SYM_FUNC_START and
> SYM_FUNC_START_ALIAS, which emit the type identifier before the
> function.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
And the reason to not make this change universally (i.e. directly in
SYM_FUNC_START) is to minimize how many of these symbol annotations get
emitted? (And to more directly indicate which asm is called indirectly?)
What happens if an asm function is called indirectly and it doesn't have
this annotation? (Is this case detectable at compile-time?)
Regardless:
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Sat, May 14, 2022 at 2:49 PM Kees Cook <[email protected]> wrote:
>
> On Fri, May 13, 2022 at 01:21:45PM -0700, Sami Tolvanen wrote:
> > With CONFIG_CFI_CLANG, assembly functions called indirectly
> > from C code must be annotated with type identifiers to pass CFI
> > checking. The compiler emits a __kcfi_typeid_<function> symbol for
> > each address-taken function declaration in C, which contains the
> > expected type identifier. Add typed versions of SYM_FUNC_START and
> > SYM_FUNC_START_ALIAS, which emit the type identifier before the
> > function.
> >
> > Signed-off-by: Sami Tolvanen <[email protected]>
>
> And the reason to not make this change universally (i.e. directly in
> SYM_FUNC_START) is to minimize how many of these symbol annotations get
> emitted? (And to more directly indicate which asm is called indirectly?)
The reason not to add this to SYM_FUNC_START is that the compiler
doesn't emit the type symbols for all functions. It currently emits
them for all address-taken function declarations in each translation
unit. We could potentially further limit this by emitting them only
for function declarations with a specific attribute, for example, but
that's something we can optimize later.
> What happens if an asm function is called indirectly and it doesn't have
> this annotation?
It will fail the CFI check.
> (Is this case detectable at compile-time?)
It's not. I'll update the commit message in the next version to
clarify these points.
Sami
On Mon, May 16, 2022 at 5:28 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 14/05/2022 23.49, Kees Cook wrote:
> > On Fri, May 13, 2022 at 01:21:45PM -0700, Sami Tolvanen wrote:
> >> With CONFIG_CFI_CLANG, assembly functions called indirectly
> >> from C code must be annotated with type identifiers to pass CFI
> >> checking. The compiler emits a __kcfi_typeid_<function> symbol for
> >> each address-taken function declaration in C, which contains the
> >> expected type identifier. Add typed versions of SYM_FUNC_START and
> >> SYM_FUNC_START_ALIAS, which emit the type identifier before the
> >> function.
> >>
> >> Signed-off-by: Sami Tolvanen <[email protected]>
> >
> > And the reason to not make this change universally (i.e. directly in
> > SYM_FUNC_START) is to minimize how many of these symbol annotations get
> > emitted? (And to more directly indicate which asm is called indirectly?)
> >
> > What happens if an asm function is called indirectly and it doesn't have
> > this annotation?
>
> Presumably that's a fail.
>
> I'm also interested in how this works at the asm/linker level. I assume
> that the .o file generated from the asm input has
> __kcfi_typeid_<function> as an undefined symbol; the compiler emits that
> symbol as an absolute one upon taking the address of <function>, and the
> linker then has the info it needs to patch things up.
Correct. The generated code looks like this:
00000000000003f7 <__cfi_blowfish_dec_blk>:
3f7: cc int3
3f8: cc int3
3f9: 8b 04 25 00 00 00 00 mov 0x0,%eax
3fc: R_X86_64_32S __kcfi_typeid_blowfish_dec_blk
400: cc int3
401: cc int3
0000000000000402 <blowfish_dec_blk>:
And the symbol table in the file that takes the address has this:
45: ffffffffef478db5 0 NOTYPE WEAK DEFAULT ABS
__kcfi_typeid_blowfish_dec_blk
> But what then happens if we have some function implemented in assembly
> which for whatever .config reason never has its address taken in any .c
> translation unit that gets linked in? Does the __kcfi_typeid_<function>
> symbol silently resolve to 0, or does the link fail?
It will fail to link in that case.
> I can't really imagine the compiler emitting __kcfi_typeid_<function>
> symbols for each and every function it sees merely declared in some header.
The compiler emits these only for address-taken declarations.
> Two different .c files both taking the address of <function> should of
> course emit the same value for __kcfi_typeid_<function>. Is there any
> sanity check anywhere that that's actually the case?
Not at the moment. I suppose we could warn about mismatches in the
linker though.
> Can we please have some objdump/readelf output from some .o files
> involved here?
Sure, I'll add examples to the commit message.
Sami
On 14/05/2022 23.49, Kees Cook wrote:
> On Fri, May 13, 2022 at 01:21:45PM -0700, Sami Tolvanen wrote:
>> With CONFIG_CFI_CLANG, assembly functions called indirectly
>> from C code must be annotated with type identifiers to pass CFI
>> checking. The compiler emits a __kcfi_typeid_<function> symbol for
>> each address-taken function declaration in C, which contains the
>> expected type identifier. Add typed versions of SYM_FUNC_START and
>> SYM_FUNC_START_ALIAS, which emit the type identifier before the
>> function.
>>
>> Signed-off-by: Sami Tolvanen <[email protected]>
>
> And the reason to not make this change universally (i.e. directly in
> SYM_FUNC_START) is to minimize how many of these symbol annotations get
> emitted? (And to more directly indicate which asm is called indirectly?)
>
> What happens if an asm function is called indirectly and it doesn't have
> this annotation?
Presumably that's a fail.
I'm also interested in how this works at the asm/linker level. I assume
that the .o file generated from the asm input has
__kcfi_typeid_<function> as an undefined symbol; the compiler emits that
symbol as an absolute one upon taking the address of <function>, and the
linker then has the info it needs to patch things up.
But what then happens if we have some function implemented in assembly
which for whatever .config reason never has its address taken in any .c
translation unit that gets linked in? Does the __kcfi_typeid_<function>
symbol silently resolve to 0, or does the link fail?
I can't really imagine the compiler emitting __kcfi_typeid_<function>
symbols for each and every function it sees merely declared in some header.
Two different .c files both taking the address of <function> should of
course emit the same value for __kcfi_typeid_<function>. Is there any
sanity check anywhere that that's actually the case?
Can we please have some objdump/readelf output from some .o files
involved here?
Rasmus