| Message ID | 20220513202159.1550547-8-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KCFI support | expand |
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 <samitolvanen@google.com> 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 <keescook@chromium.org>
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 <samitolvanen@google.com> > > 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
On Sat, May 14, 2022 at 2:49 PM Kees Cook <keescook@chromium.org> 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 <samitolvanen@google.com> > > 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 <linux@rasmusvillemoes.dk> 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 <samitolvanen@google.com> > > > > 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
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 */
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 <samitolvanen@google.com> --- include/linux/cfi_types.h | 57 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 include/linux/cfi_types.h