On Thu, Apr 10, 2025 at 03:25:22PM +0200, Peter Zijlstra wrote:

> I should get objtool to warn about those. They undermine the point of
> CFI.

---
Subject: objtool: Detect __nocfi calls

Detect and WARN about no_sanitize(kcfi) indirect calls.

Apparently there were a few in some Rust 'core' that got included in the
kernel and things went *bang*.

This is not a supported form for kernel code. So detect and warn about
it.

Adds an annotation for the two cases where we have to live with them:

 - EFI stubs;
 - kexec handover.

Notably, EFI calls fully disable IBT, as such using runtime EFI services
is a significant security issue. If you can exploit the kexec handover,
you get to keep it.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index cc73f9708464..67ba0db92272 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -442,6 +442,7 @@ void __nocfi machine_kexec(struct kimage *image)
 
 	__ftrace_enabled_restore(save_ftrace_enabled);
 }
+ANNOTATE_NOCFI_SYM(machine_kexec);
 
 /* arch-dependent functionality related to kexec file-based syscall */
 
diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S
index 2206b8bc47b8..2bee139ae1ee 100644
--- a/arch/x86/platform/efi/efi_stub_64.S
+++ b/arch/x86/platform/efi/efi_stub_64.S
@@ -11,6 +11,7 @@
 #include <asm/nospec-branch.h>
 
 SYM_FUNC_START(__efi_call)
+	ANNOTATE_NOCFI
 	pushq %rbp
 	movq %rsp, %rbp
 	and $~0xf, %rsp
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 366ad004d794..518daea19699 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -185,6 +185,8 @@
  */
 #define ANNOTATE_REACHABLE(label)	__ASM_ANNOTATE(label, ANNOTYPE_REACHABLE)
 
+#define ANNOTATE_NOCFI_SYM(sym)		asm(__ASM_ANNOTATE(sym, ANNOTYPE_NOCFI))
+
 #else
 #define ANNOTATE_NOENDBR		ANNOTATE type=ANNOTYPE_NOENDBR
 #define ANNOTATE_RETPOLINE_SAFE		ANNOTATE type=ANNOTYPE_RETPOLINE_SAFE
@@ -194,6 +196,7 @@
 #define ANNOTATE_INTRA_FUNCTION_CALL	ANNOTATE type=ANNOTYPE_INTRA_FUNCTION_CALL
 #define ANNOTATE_UNRET_BEGIN		ANNOTATE type=ANNOTYPE_UNRET_BEGIN
 #define ANNOTATE_REACHABLE		ANNOTATE type=ANNOTYPE_REACHABLE
+#define ANNOTATE_NOCFI			ANNOTATE type=ANNOTYPE_NOCFI
 #endif
 
 #if defined(CONFIG_NOINSTR_VALIDATION) && \
diff --git a/include/linux/objtool_types.h b/include/linux/objtool_types.h
index df5d9fa84dba..aceac94632c8 100644
--- a/include/linux/objtool_types.h
+++ b/include/linux/objtool_types.h
@@ -65,5 +65,6 @@ struct unwind_hint {
 #define ANNOTYPE_IGNORE_ALTS		6
 #define ANNOTYPE_INTRA_FUNCTION_CALL	7
 #define ANNOTYPE_REACHABLE		8
+#define ANNOTYPE_NOCFI			9
 
 #endif /* _LINUX_OBJTOOL_TYPES_H */
diff --git a/tools/include/linux/objtool_types.h b/tools/include/linux/objtool_types.h
index df5d9fa84dba..aceac94632c8 100644
--- a/tools/include/linux/objtool_types.h
+++ b/tools/include/linux/objtool_types.h
@@ -65,5 +65,6 @@ struct unwind_hint {
 #define ANNOTYPE_IGNORE_ALTS		6
 #define ANNOTYPE_INTRA_FUNCTION_CALL	7
 #define ANNOTYPE_REACHABLE		8
+#define ANNOTYPE_NOCFI			9
 
 #endif /* _LINUX_OBJTOOL_TYPES_H */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4a1f6c3169b3..868601760953 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1382,6 +1382,26 @@ static int add_call_dest(struct objtool_file *file, struct instruction *insn,
 
 static int add_retpoline_call(struct objtool_file *file, struct instruction *insn)
 {
+	struct symbol *sym = insn->sym;
+
+	/*
+	 * kCFI call sites look like:
+	 *
+	 *     movl $(-0x12345678), %r10d
+	 *     addl -4(%r11), %r10d
+	 *     jz 1f
+	 *     ud2
+	 *  1: cs call __x86_indirect_thunk_r11
+	 *
+	 * Verify all indirect calls are kCFI adorned by checking for the UD2.
+	 * Notably, doing __nocfi calls to regular (cfi) functions is broken.
+	 */
+	if (opts.cfi && sym && sym->type == STT_FUNC && !sym->nocfi) {
+		struct instruction *prev = prev_insn_same_sym(file, insn);
+		if (!prev || prev->type != INSN_BUG)
+			WARN_INSN(insn, "no-cfi indirect call!");
+	}
+
 	/*
 	 * Retpoline calls/jumps are really dynamic calls/jumps in disguise,
 	 * so convert them accordingly.
@@ -2334,6 +2354,8 @@ static int read_annotate(struct objtool_file *file,
 
 static int __annotate_early(struct objtool_file *file, int type, struct instruction *insn)
 {
+	struct symbol *sym;
+
 	switch (type) {
 
 	/* Must be before add_special_section_alts() */
@@ -2348,6 +2370,19 @@ static int __annotate_early(struct objtool_file *file, int type, struct instruct
 		insn->noendbr = 1;
 		break;
 
+	/*
+	 * Must be before add_{jump,call}_destination(), specifically any
+	 * add_retpoline_call().
+	 */
+	case ANNOTYPE_NOCFI:
+		sym = insn->sym;
+		if (!sym) {
+			WARN_INSN(insn, "dodgy NOCFI annotation");
+			break;
+		}
+		insn->sym->nocfi = 1;
+		break;
+
 	default:
 		break;
 	}
@@ -2428,6 +2463,10 @@ static int __annotate_late(struct objtool_file *file, int type, struct instructi
 		insn->dead_end = false;
 		break;
 
+	case ANNOTYPE_NOCFI:
+		/* early */
+		break;
+
 	default:
 		ERROR_INSN(insn, "Unknown annotation type: %d", type);
 		return -1;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index c7c4e87ebe88..f60604d30793 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -70,6 +70,7 @@ struct symbol {
 	u8 local_label       : 1;
 	u8 frame_pointer     : 1;
 	u8 ignore	     : 1;
+	u8 nocfi             : 1;
 	struct list_head pv_target;
 	struct reloc *relocs;
 };

Hi Peter,

On Thu, Apr 10, 2025 at 05:45:56PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 10, 2025 at 03:25:22PM +0200, Peter Zijlstra wrote:
> 
> > I should get objtool to warn about those. They undermine the point of
> > CFI.
> 
> ---
> Subject: objtool: Detect __nocfi calls
> 
> Detect and WARN about no_sanitize(kcfi) indirect calls.
> 
> Apparently there were a few in some Rust 'core' that got included in the
> kernel and things went *bang*.
> 
> This is not a supported form for kernel code. So detect and warn about
> it.

Cool, this looks useful!

> Adds an annotation for the two cases where we have to live with them:
> 
>  - EFI stubs;
>  - kexec handover.
> 
> Notably, EFI calls fully disable IBT, as such using runtime EFI services
> is a significant security issue. If you can exploit the kexec handover,
> you get to keep it.

OK, with this applied I now see a warning about the __nocfi call in
Rust code:

vmlinux.o: warning: objtool: _RNvNtCsjWi3sh0wSlE_4core3fmt5write+0x170: no-cfi indirect call!

But an allmodconfig build reveals a few more warnings:

arch/x86/kvm/kvm.o: warning: objtool: x86_emulate_insn+0xaf7: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_das+0x290: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_imul_3op+0x15f: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_aam+0x21c: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_aad+0x1dc: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_loop+0x312: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_cmpxchg+0x329: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_bsf_c+0x1b7: no-cfi indirect call!
arch/x86/kvm/kvm.o: warning: objtool: em_bsr_c+0x1b7: no-cfi indirect call!
arch/x86/kvm/kvm-intel.o: warning: objtool: vmx_do_interrupt_irqoff+0xe: no-cfi indirect call!
drivers/misc/lkdtm/lkdtm.o: warning: objtool: execute_location+0x5a: no-cfi indirect call!
drivers/pci/controller/pci-hyperv.o: warning: objtool: hv_do_hypercall+0x150: no-cfi indirect call!
drivers/hv/hv_balloon.o: warning: objtool: hv_free_page_report+0x5da: no-cfi indirect call!
drivers/hv/hv_vmbus.o: warning: objtool: hv_post_message+0x457: no-cfi indirect call!
drivers/hv/hv_vmbus.o: warning: objtool: vmbus_set_event+0x2a2: no-cfi indirect call!
vmlinux.o: warning: objtool: hyperv_flush_tlb_multi+0xe96: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_do_hypercall+0x12b: no-cfi indirect call!
vmlinux.o: warning: objtool: hyperv_flush_guest_mapping+0x2e3: no-cfi indirect call!
vmlinux.o: warning: objtool: hyperv_flush_guest_mapping_range+0x36a: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_do_hypercall+0x150: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_snp_boot_ap+0xb08: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_vtom_set_host_visibility+0x54a: no-cfi indirect call!
vmlinux.o: warning: objtool: __send_ipi_one+0x362: no-cfi indirect call!
vmlinux.o: warning: objtool: __send_ipi_mask_ex+0x655: no-cfi indirect call!
vmlinux.o: warning: objtool: __send_ipi_mask+0x635: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_do_hypercall+0x150: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_query_ext_cap+0x175: no-cfi indirect call!
vmlinux.o: warning: objtool: get_vtl+0x38c: no-cfi indirect call!
vmlinux.o: warning: objtool: hv_get_partition_id+0x224: no-cfi indirect call!

Sami

On Thu, Apr 10, 2025 at 07:43:34PM +0000, Sami Tolvanen wrote:

> But an allmodconfig build reveals a few more warnings:

Sigh, let me go look at all that and address Josh's feedback.

Thanks!

On Fri, Apr 11, 2025 at 08:44:49AM +0200, Peter Zijlstra wrote:
> On Thu, Apr 10, 2025 at 07:43:34PM +0000, Sami Tolvanen wrote:
> 
> > But an allmodconfig build reveals a few more warnings:
> 
> Sigh, let me go look at all that and address Josh's feedback.

Latest sits here:

git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/core

I'll have the robots chew on it over the weekend and then post on
Monday.

On Thu, Apr 10, 2025 at 5:46 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Detect and WARN about no_sanitize(kcfi) indirect calls.
>
> Apparently there were a few in some Rust 'core' that got included in the
> kernel and things went *bang*.
>
> This is not a supported form for kernel code. So detect and warn about
> it.

That was quick!

It seems to work -- I got:

      LD      vmlinux.o
    vmlinux.o: warning: objtool:
_RNvNtCs8DPF7ip8WBQ_4core3fmt5write+0x170: no-cfi indirect call!

Cheers,
Miguel

On Thu, Apr 10, 2025 at 05:45:56PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 10, 2025 at 03:25:22PM +0200, Peter Zijlstra wrote:
> 
> > I should get objtool to warn about those. They undermine the point of
> > CFI.
> 
> ---
> Subject: objtool: Detect __nocfi calls

"Warn on indirect calls in __nocfi functions" ?

>  static int add_retpoline_call(struct objtool_file *file, struct instruction *insn)
>  {
> +	struct symbol *sym = insn->sym;
> +
> +	/*
> +	 * kCFI call sites look like:
> +	 *
> +	 *     movl $(-0x12345678), %r10d
> +	 *     addl -4(%r11), %r10d
> +	 *     jz 1f
> +	 *     ud2
> +	 *  1: cs call __x86_indirect_thunk_r11
> +	 *
> +	 * Verify all indirect calls are kCFI adorned by checking for the UD2.
> +	 * Notably, doing __nocfi calls to regular (cfi) functions is broken.
> +	 */
> +	if (opts.cfi && sym && sym->type == STT_FUNC && !sym->nocfi) {
> +		struct instruction *prev = prev_insn_same_sym(file, insn);
> +		if (!prev || prev->type != INSN_BUG)
> +			WARN_INSN(insn, "no-cfi indirect call!");

Since this can break things pretty badly at runtime, this should
actually fail the build on CONFIG_OBJTOOL_WERROR.

The warning counts aren't plumbed in this early, so can this check be
done later?  validate_retpoline() or validate_call()?

-- 
Josh

On Thu, Apr 10, 2025 at 12:09:02PM -0700, Josh Poimboeuf wrote:
> On Thu, Apr 10, 2025 at 05:45:56PM +0200, Peter Zijlstra wrote:
> > On Thu, Apr 10, 2025 at 03:25:22PM +0200, Peter Zijlstra wrote:
> > 
> > > I should get objtool to warn about those. They undermine the point of
> > > CFI.
> > 
> > ---
> > Subject: objtool: Detect __nocfi calls
> 
> "Warn on indirect calls in __nocfi functions" ?

Yeah, I suppose that's more accurate.

> >  static int add_retpoline_call(struct objtool_file *file, struct instruction *insn)
> >  {
> > +	struct symbol *sym = insn->sym;
> > +
> > +	/*
> > +	 * kCFI call sites look like:
> > +	 *
> > +	 *     movl $(-0x12345678), %r10d
> > +	 *     addl -4(%r11), %r10d
> > +	 *     jz 1f
> > +	 *     ud2
> > +	 *  1: cs call __x86_indirect_thunk_r11
> > +	 *
> > +	 * Verify all indirect calls are kCFI adorned by checking for the UD2.
> > +	 * Notably, doing __nocfi calls to regular (cfi) functions is broken.
> > +	 */
> > +	if (opts.cfi && sym && sym->type == STT_FUNC && !sym->nocfi) {
> > +		struct instruction *prev = prev_insn_same_sym(file, insn);
> > +		if (!prev || prev->type != INSN_BUG)
> > +			WARN_INSN(insn, "no-cfi indirect call!");
> 
> Since this can break things pretty badly at runtime, this should
> actually fail the build on CONFIG_OBJTOOL_WERROR.

Oh right, I still got to adjust to the new world order here :-)

> The warning counts aren't plumbed in this early, so can this check be
> done later?  validate_retpoline() or validate_call()?

Hmm, let me have a poke around, see what can be done.