| Message ID | 20211013181658.1020262-10-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | x86: Add support for Clang CFI | expand |
On Wed, Oct 13, 2021 at 11:16:52AM -0700, Sami Tolvanen wrote: > The kernel has several assembly functions that are not directly callable > from C. Use an opaque type for these function prototypes to make misuse > harder, and to avoid the need to annotate references to these functions > for Clang's Control-Flow Integrity (CFI). > > Suggested-by: Andy Lutomirski <luto@amacapital.net> > Suggested-by: Alexander Lobakin <alobakin@pm.me> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > --- > arch/x86/include/asm/ftrace.h | 2 +- > arch/x86/include/asm/idtentry.h | 10 +++++----- > arch/x86/include/asm/page_64.h | 7 ++++--- > arch/x86/include/asm/paravirt_types.h | 3 ++- > arch/x86/include/asm/processor.h | 2 +- > arch/x86/include/asm/proto.h | 25 +++++++++++++------------ > arch/x86/include/asm/uaccess_64.h | 9 +++------ > arch/x86/kernel/alternative.c | 2 +- > arch/x86/kernel/ftrace.c | 2 +- > arch/x86/kernel/paravirt.c | 4 ++-- > arch/x86/kvm/emulate.c | 4 ++-- > arch/x86/kvm/kvm_emulate.h | 9 ++------- > arch/x86/xen/enlighten_pv.c | 6 +++--- > arch/x86/xen/xen-ops.h | 10 +++++----- > 14 files changed, 45 insertions(+), 50 deletions(-) No matter from which direction I look at it, wrapping some functions which a crazy macro doesn't look good. So what's the plan here? Everytime someone adds an asm function which is not callable from C but forgets to use that magic macro, someone from team CFI will send a patch fixing that? I.e., a whack-a-mole game? If we're going to do that keep-CFI-working game, we might just as well not do the macro but use the C code it evaluates to, so that at least it looks ok-ish: DECLARE_NOT_CALLED_FROM_C(int3_magic); vs extern const u8 int3_magic[]; (Just picked one at random). Thx.
On Thu, Oct 14, 2021 at 01:21:38PM +0200, Borislav Petkov wrote: > On Wed, Oct 13, 2021 at 11:16:52AM -0700, Sami Tolvanen wrote: > > The kernel has several assembly functions that are not directly callable > > from C. Use an opaque type for these function prototypes to make misuse > > harder, and to avoid the need to annotate references to these functions > > for Clang's Control-Flow Integrity (CFI). > > > > Suggested-by: Andy Lutomirski <luto@amacapital.net> > > Suggested-by: Alexander Lobakin <alobakin@pm.me> > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > --- > > arch/x86/include/asm/ftrace.h | 2 +- > > arch/x86/include/asm/idtentry.h | 10 +++++----- > > arch/x86/include/asm/page_64.h | 7 ++++--- > > arch/x86/include/asm/paravirt_types.h | 3 ++- > > arch/x86/include/asm/processor.h | 2 +- > > arch/x86/include/asm/proto.h | 25 +++++++++++++------------ > > arch/x86/include/asm/uaccess_64.h | 9 +++------ > > arch/x86/kernel/alternative.c | 2 +- > > arch/x86/kernel/ftrace.c | 2 +- > > arch/x86/kernel/paravirt.c | 4 ++-- > > arch/x86/kvm/emulate.c | 4 ++-- > > arch/x86/kvm/kvm_emulate.h | 9 ++------- > > arch/x86/xen/enlighten_pv.c | 6 +++--- > > arch/x86/xen/xen-ops.h | 10 +++++----- > > 14 files changed, 45 insertions(+), 50 deletions(-) > > No matter from which direction I look at it, wrapping some functions > which a crazy macro doesn't look good. > > So what's the plan here? > > Everytime someone adds an asm function which is not callable from C but > forgets to use that magic macro, someone from team CFI will send a patch > fixing that? > > I.e., a whack-a-mole game? I don't think it's a super common thing to add, so in this case, yes, I think doing it on a case-by-case basis will be fine. This is common practice in the kernel; not everyone tests all CONFIGs, so stuff gets missed, patches are sent, life goes on. :) > If we're going to do that keep-CFI-working game, we might just as well > not do the macro but use the C code it evaluates to, so that at least it > looks ok-ish: > > DECLARE_NOT_CALLED_FROM_C(int3_magic); > > vs > > extern const u8 int3_magic[]; I'd _much_ prefer keeping the macro, as it explains what's going on, which doesn't require a comment at every "extern const u8 foo[]" usage. It serves as an annotation, etc. And, there's been a lot of discussion on the best way to do this, what to name it, etc. This looks to be the best option currently. -Kees
On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote: > I don't think it's a super common thing to add, so in this case, yes, > I think doing it on a case-by-case basis will be fine. You don't have a choice - if there's no automation which verifies whether all the CFI annotation needed is there, people won't know what to wrap in what macro. > I'd _much_ prefer keeping the macro, as it explains what's going on, > which doesn't require a comment at every "extern const u8 foo[]" usage. You don't have to - it is just an extern. > It serves as an annotation, etc. Oh, that I figured. > And, there's been a lot of discussion on the best way to do this, what > to name it, etc. Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more sense to me even if it doesn't specify the aspect that it is not called by C but who cares - it is generic enough. > This looks to be the best option currently. Maybe because wrapping some random symbols in a obfuscating macro to make the next tool happy, is simply the wrong thing to do. I know, I know, clang CFI needs it because of magical reason X but that doesn't make it any better. Someday soon we'll have to write a tutorial for people submitting kernel patches explaining what annotation to add where and why. Why can't clang be taught to ignore those symbols: clang -fsanitize=cfi -fsanitize-cfi-ignore-symbols=<list> ? Hmm, looking at https://clang.llvm.org/docs/ControlFlowIntegrity.html there *is* an ignore list: "Ignorelist A Sanitizer special case list can be used to relax CFI checks for certain source files, functions and types using the src, fun and type entity types. Specific CFI modes can be be specified using [section] headers. ... # Ignore all functions with names containing MyFooBar. fun:*MyFooBar* ..." So why aren't we doing that instead of those macros?
On Thu, Oct 14, 2021 at 10:31 AM Borislav Petkov <bp@alien8.de> wrote: > > On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote: > > I don't think it's a super common thing to add, so in this case, yes, > > I think doing it on a case-by-case basis will be fine. > > You don't have a choice - if there's no automation which verifies > whether all the CFI annotation needed is there, people won't know what > to wrap in what macro. > > > I'd _much_ prefer keeping the macro, as it explains what's going on, > > which doesn't require a comment at every "extern const u8 foo[]" usage. > > You don't have to - it is just an extern. > > > It serves as an annotation, etc. > > Oh, that I figured. > > > And, there's been a lot of discussion on the best way to do this, what > > to name it, etc. > > Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more > sense to me even if it doesn't specify the aspect that it is not called > by C but who cares - it is generic enough. > > > This looks to be the best option currently. > > Maybe because wrapping some random symbols in a obfuscating macro to > make the next tool happy, is simply the wrong thing to do. I know, I > know, clang CFI needs it because of magical reason X but that doesn't > make it any better. Someday soon we'll have to write a tutorial for > people submitting kernel patches explaining what annotation to add where > and why. > > Why can't clang be taught to ignore those symbols: > > clang -fsanitize=cfi -fsanitize-cfi-ignore-symbols=<list> > > ? > > Hmm, looking at https://clang.llvm.org/docs/ControlFlowIntegrity.html > > there *is* an ignore list: > > "Ignorelist > > A Sanitizer special case list can be used to relax CFI checks for > certain source files, functions and types using the src, fun and type > entity types. Specific CFI modes can be be specified using [section] > headers. > > ... > > # Ignore all functions with names containing MyFooBar. > fun:*MyFooBar* > ..." > > > So why aren't we doing that instead of those macros? The ignorelist can be used to disable CFI checking in the listed functions, but the compiler still checks indirect calls made to these functions from elsewhere, and therefore, using an opaque type is still necessary to ensure the symbol address isn't replaced with a jump table address. Anyway, I thought using a macro would make the code easier to understand. I'm happy to rename it to something that makes more sense, but also fine with switching to a simple extern u8[] if that's preferable. Sami
On Thu, Oct 14, 2021 at 07:31:26PM +0200, Borislav Petkov wrote: > On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote: > Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more > sense to me even if it doesn't specify the aspect that it is not called > by C but who cares - it is generic enough. Around we go. :) Josh[1] and Steven[2] explicitly disagreed with that name, leading to the current name[3]. Do you want it to be DECLARE_ASM_FUNC_SYMBOL() over those objections? I'd really like to finish this shed -- I need to take the bikes in from the rain. :P -Kees [1] https://lore.kernel.org/lkml/20211006032945.axlqh3vehgar6adr@treble/ [2] https://lore.kernel.org/lkml/20211006101654.6a5be402@gandalf.local.home/ [3] https://lore.kernel.org/lkml/CABCJKufCaOXOUF43a-PQshO8aEsMNhZ2EiyGMSOp9ZGn57G=pg@mail.gmail.com/
On Thu, 14 Oct 2021 11:47:01 -0700 Kees Cook <keescook@chromium.org> wrote: > On Thu, Oct 14, 2021 at 07:31:26PM +0200, Borislav Petkov wrote: > > On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote: > > Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more > > sense to me even if it doesn't specify the aspect that it is not called > > by C but who cares - it is generic enough. > > Around we go. :) Josh[1] and Steven[2] explicitly disagreed with > that name, leading to the current name[3]. Do you want it to be > DECLARE_ASM_FUNC_SYMBOL() over those objections? Just note, that I was fine with the original name, but was against the version Josh suggested ;-) > > I'd really like to finish this shed -- I need to take the bikes in from > the rain. :P Back to black it is! -- Steve
On Thu, Oct 14, 2021 at 11:24 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > Anyway, I thought using a macro would make the code easier to > understand. I'm happy to rename it to something that makes more sense, > but also fine with switching to a simple extern u8[] if that's > preferable. Perhaps `extern u8 []`s with a comment that these symbols are functions that aren't meant to be called from C, only asm (or compiler generated code) would suffice?
On Thu, Oct 14, 2021 at 02:52:11PM -0400, Steven Rostedt wrote: > On Thu, 14 Oct 2021 11:47:01 -0700 > Kees Cook <keescook@chromium.org> wrote: > > > On Thu, Oct 14, 2021 at 07:31:26PM +0200, Borislav Petkov wrote: > > > On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote: > > > Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more > > > sense to me even if it doesn't specify the aspect that it is not called > > > by C but who cares - it is generic enough. > > > > Around we go. :) Josh[1] and Steven[2] explicitly disagreed with > > that name, leading to the current name[3]. Do you want it to be > > DECLARE_ASM_FUNC_SYMBOL() over those objections? > > Just note, that I was fine with the original name, but was against the > version Josh suggested ;-) Naming is important, especially for something as confusing as this. We need to be able to read it in a few months and have some idea of what's going on. "DECLARE_ASM_FUNC_SYMBOL" is nonsensical. As a reader of the code I wonder why are some asm functions using it and not others? And how do I know if I need it for my new function? "extern const u8 int3_magic[]" is even worse. Why are some asm functions randomly declared as arrays, and others not? Where can I go to find out more without digging through the commit log?
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h index 9f3130f40807..bc675d6ce4eb 100644 --- a/arch/x86/include/asm/ftrace.h +++ b/arch/x86/include/asm/ftrace.h @@ -17,7 +17,7 @@ #ifndef __ASSEMBLY__ extern atomic_t modifying_ftrace_code; -extern void __fentry__(void); +DECLARE_NOT_CALLED_FROM_C(__fentry__); static inline unsigned long ftrace_call_adjust(unsigned long addr) { diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h index 1345088e9902..6538bf5a47d6 100644 --- a/arch/x86/include/asm/idtentry.h +++ b/arch/x86/include/asm/idtentry.h @@ -27,8 +27,8 @@ * as well which is used to emit the entry stubs in entry_32/64.S. */ #define DECLARE_IDTENTRY(vector, func) \ - asmlinkage void asm_##func(void); \ - asmlinkage void xen_asm_##func(void); \ + DECLARE_NOT_CALLED_FROM_C(asm_##func); \ + DECLARE_NOT_CALLED_FROM_C(xen_asm_##func); \ __visible void func(struct pt_regs *regs) /** @@ -78,8 +78,8 @@ static __always_inline void __##func(struct pt_regs *regs) * C-handler. */ #define DECLARE_IDTENTRY_ERRORCODE(vector, func) \ - asmlinkage void asm_##func(void); \ - asmlinkage void xen_asm_##func(void); \ + DECLARE_NOT_CALLED_FROM_C(asm_##func); \ + DECLARE_NOT_CALLED_FROM_C(xen_asm_##func); \ __visible void func(struct pt_regs *regs, unsigned long error_code) /** @@ -386,7 +386,7 @@ static __always_inline void __##func(struct pt_regs *regs) * - The C handler called from the C shim */ #define DECLARE_IDTENTRY_DF(vector, func) \ - asmlinkage void asm_##func(void); \ + DECLARE_NOT_CALLED_FROM_C(asm_##func); \ __visible void func(struct pt_regs *regs, \ unsigned long error_code, \ unsigned long address) diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h index 4bde0dc66100..22beb80c0708 100644 --- a/arch/x86/include/asm/page_64.h +++ b/arch/x86/include/asm/page_64.h @@ -5,6 +5,7 @@ #include <asm/page_64_types.h> #ifndef __ASSEMBLY__ +#include <linux/linkage.h> #include <asm/alternative.h> /* duplicated to the one in bootmem.h */ @@ -40,9 +41,9 @@ extern unsigned long __phys_addr_symbol(unsigned long); #define pfn_valid(pfn) ((pfn) < max_pfn) #endif -void clear_page_orig(void *page); -void clear_page_rep(void *page); -void clear_page_erms(void *page); +DECLARE_NOT_CALLED_FROM_C(clear_page_orig); +DECLARE_NOT_CALLED_FROM_C(clear_page_rep); +DECLARE_NOT_CALLED_FROM_C(clear_page_erms); static inline void clear_page(void *page) { diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h index d9d6b0203ec4..dfaa50d20d6a 100644 --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -38,6 +38,7 @@ #include <asm/desc_defs.h> #include <asm/pgtable_types.h> #include <asm/nospec-branch.h> +#include <asm/proto.h> struct page; struct thread_struct; @@ -271,7 +272,7 @@ struct paravirt_patch_template { extern struct pv_info pv_info; extern struct paravirt_patch_template pv_ops; -extern void (*paravirt_iret)(void); +extern asm_func_ptr paravirt_iret; #define PARAVIRT_PATCH(x) \ (offsetof(struct paravirt_patch_template, x) / sizeof(void *)) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 577f342dbfb2..1e6b6372b53b 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -449,7 +449,7 @@ static inline unsigned long cpu_kernelmode_gs_base(int cpu) DECLARE_PER_CPU(void *, hardirq_stack_ptr); DECLARE_PER_CPU(bool, hardirq_stack_inuse); -extern asmlinkage void ignore_sysret(void); +DECLARE_NOT_CALLED_FROM_C(ignore_sysret); /* Save actual FS/GS selectors and bases to current->thread */ void current_save_fsgs(void); diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h index 8c5d1910a848..55d1161c985a 100644 --- a/arch/x86/include/asm/proto.h +++ b/arch/x86/include/asm/proto.h @@ -2,6 +2,7 @@ #ifndef _ASM_X86_PROTO_H #define _ASM_X86_PROTO_H +#include <linux/linkage.h> #include <asm/ldt.h> struct task_struct; @@ -11,26 +12,26 @@ struct task_struct; void syscall_init(void); #ifdef CONFIG_X86_64 -void entry_SYSCALL_64(void); -void entry_SYSCALL_64_safe_stack(void); +DECLARE_NOT_CALLED_FROM_C(entry_SYSCALL_64); +DECLARE_NOT_CALLED_FROM_C(entry_SYSCALL_64_safe_stack); long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2); #endif #ifdef CONFIG_X86_32 -void entry_INT80_32(void); -void entry_SYSENTER_32(void); -void __begin_SYSENTER_singlestep_region(void); -void __end_SYSENTER_singlestep_region(void); +DECLARE_NOT_CALLED_FROM_C(entry_INT80_32); +DECLARE_NOT_CALLED_FROM_C(entry_SYSENTER_32); +DECLARE_NOT_CALLED_FROM_C(__begin_SYSENTER_singlestep_region); +DECLARE_NOT_CALLED_FROM_C(__end_SYSENTER_singlestep_region); #endif #ifdef CONFIG_IA32_EMULATION -void entry_SYSENTER_compat(void); -void __end_entry_SYSENTER_compat(void); -void entry_SYSCALL_compat(void); -void entry_SYSCALL_compat_safe_stack(void); -void entry_INT80_compat(void); +DECLARE_NOT_CALLED_FROM_C(entry_SYSENTER_compat); +DECLARE_NOT_CALLED_FROM_C(__end_entry_SYSENTER_compat); +DECLARE_NOT_CALLED_FROM_C(entry_SYSCALL_compat); +DECLARE_NOT_CALLED_FROM_C(entry_SYSCALL_compat_safe_stack); +DECLARE_NOT_CALLED_FROM_C(entry_INT80_compat); #ifdef CONFIG_XEN_PV -void xen_entry_INT80_compat(void); +DECLARE_NOT_CALLED_FROM_C(xen_entry_INT80_compat); #endif #endif diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h index 45697e04d771..96cf72d6b75c 100644 --- a/arch/x86/include/asm/uaccess_64.h +++ b/arch/x86/include/asm/uaccess_64.h @@ -17,12 +17,9 @@ */ /* Handles exceptions in both to and from, but doesn't do access_ok */ -__must_check unsigned long -copy_user_enhanced_fast_string(void *to, const void *from, unsigned len); -__must_check unsigned long -copy_user_generic_string(void *to, const void *from, unsigned len); -__must_check unsigned long -copy_user_generic_unrolled(void *to, const void *from, unsigned len); +DECLARE_NOT_CALLED_FROM_C(copy_user_enhanced_fast_string); +DECLARE_NOT_CALLED_FROM_C(copy_user_generic_string); +DECLARE_NOT_CALLED_FROM_C(copy_user_generic_unrolled); static __always_inline __must_check unsigned long copy_user_generic(void *to, const void *from, unsigned len) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index e9da3dc71254..1a07ce172667 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -530,7 +530,7 @@ extern struct paravirt_patch_site __start_parainstructions[], * convention such that we can 'call' it from assembly. */ -extern void int3_magic(unsigned int *ptr); /* defined in asm */ +DECLARE_NOT_CALLED_FROM_C(int3_magic); asm ( " .pushsection .init.text, \"ax\", @progbits\n" diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 1b3ce3b4a2a2..a73dfe7c430d 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -589,7 +589,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops) #ifdef CONFIG_FUNCTION_GRAPH_TRACER #ifdef CONFIG_DYNAMIC_FTRACE -extern void ftrace_graph_call(void); +DECLARE_NOT_CALLED_FROM_C(ftrace_graph_call); static const char *ftrace_jmp_replace(unsigned long ip, unsigned long addr) { diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c index ebc45360ffd4..d3471c0e285a 100644 --- a/arch/x86/kernel/paravirt.c +++ b/arch/x86/kernel/paravirt.c @@ -138,7 +138,7 @@ void paravirt_set_sched_clock(u64 (*func)(void)) } /* These are in entry.S */ -extern void native_iret(void); +DECLARE_NOT_CALLED_FROM_C(native_iret); static struct resource reserve_ioports = { .start = 0, @@ -403,7 +403,7 @@ struct paravirt_patch_template pv_ops = { #ifdef CONFIG_PARAVIRT_XXL NOKPROBE_SYMBOL(native_load_idt); -void (*paravirt_iret)(void) = native_iret; +asm_func_ptr paravirt_iret = native_iret; #endif EXPORT_SYMBOL(pv_ops); diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 9a144ca8e146..91600a05b6fd 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -201,7 +201,7 @@ struct opcode { const struct escape *esc; const struct instr_dual *idual; const struct mode_dual *mdual; - void (*fastop)(struct fastop *fake); + fastop_t fastop; } u; int (*check_perm)(struct x86_emulate_ctxt *ctxt); }; @@ -322,7 +322,7 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop); __FOP_RET(#name) #define FOP_START(op) \ - extern void em_##op(struct fastop *fake); \ + DECLARE_NOT_CALLED_FROM_C(em_##op); \ asm(".pushsection .text, \"ax\" \n\t" \ ".global em_" #op " \n\t" \ ".align " __stringify(FASTOP_SIZE) " \n\t" \ diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h index 68b420289d7e..44c1a9324e1c 100644 --- a/arch/x86/kvm/kvm_emulate.h +++ b/arch/x86/kvm/kvm_emulate.h @@ -290,13 +290,8 @@ enum x86emul_mode { #define X86EMUL_SMM_MASK (1 << 6) #define X86EMUL_SMM_INSIDE_NMI_MASK (1 << 7) -/* - * fastop functions are declared as taking a never-defined fastop parameter, - * so they can't be called from C directly. - */ -struct fastop; - -typedef void (*fastop_t)(struct fastop *); +/* fastop functions cannot be called from C directly. */ +typedef asm_func_ptr fastop_t; struct x86_emulate_ctxt { void *vcpu; diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 4f63117f09bb..d52929ac70c7 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -584,8 +584,8 @@ DEFINE_IDTENTRY_RAW(xenpv_exc_machine_check) #endif struct trap_array_entry { - void (*orig)(void); - void (*xen)(void); + asm_func_ptr orig; + asm_func_ptr xen; bool ist_okay; }; @@ -644,7 +644,7 @@ static bool __ref get_trap_addr(void **addr, unsigned int ist) struct trap_array_entry *entry = trap_array + nr; if (*addr == entry->orig) { - *addr = entry->xen; + *addr = (void *)entry->xen; ist_okay = entry->ist_okay; found = true; break; diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h index 8bc8b72a205d..a8fbf485556b 100644 --- a/arch/x86/xen/xen-ops.h +++ b/arch/x86/xen/xen-ops.h @@ -8,12 +8,12 @@ #include <xen/xen-ops.h> /* These are code, but not functions. Defined in entry.S */ -extern const char xen_failsafe_callback[]; +DECLARE_NOT_CALLED_FROM_C(xen_failsafe_callback); -void xen_sysenter_target(void); +DECLARE_NOT_CALLED_FROM_C(xen_sysenter_target); #ifdef CONFIG_X86_64 -void xen_syscall_target(void); -void xen_syscall32_target(void); +DECLARE_NOT_CALLED_FROM_C(xen_syscall_target); +DECLARE_NOT_CALLED_FROM_C(xen_syscall32_target); #endif extern void *xen_initial_gdt; @@ -139,7 +139,7 @@ __visible unsigned long xen_read_cr2(void); __visible unsigned long xen_read_cr2_direct(void); /* These are not functions, and cannot be called normally */ -__visible void xen_iret(void); +DECLARE_NOT_CALLED_FROM_C(xen_iret); extern int xen_panic_handler_init(void);