Skip to contentSkip to conversation

objtool/arm64: Port klp-build to arm64

PATCH Superseded Josh Poimboeuf 2 participants 17 messages Mar 17 – Mar 30

Subsystems (7)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Josh Poimboeuf

author maintainer
#0 lore raw

v2 cover objtool/arm64: Port klp-build to arm64

v2:
- patches 1-2 were merged, rebase on tip/master
- improve commit message for "objtool: Extricate checksum calculation from validate_branch()"
- add review tags

v1: https://lore.kernel.org/[email protected]

Port objtool and the klp-build tooling (for building livepatch modules) to arm64.

Note this doesn't bring all the objtool bells and whistles to arm64, nor any of the CFG reverse engineering. This only adds the bare minimum needed for 'objtool --checksum'.

And note that objtool still doesn't get enabled at all for normal arm64 kernel builds, so this doesn't affect any users other than those running klp-build directly.

Josh Poimboeuf (12):
arm64: Annotate intra-function calls
arm64: head: Move boot header to .head.data
arm64: Fix EFI linking with -fdata-sections
crypto: arm64: Move data to .rodata
objtool: Extricate checksum calculation from validate_branch()
objtool: Allow setting --mnop without --mcount
kbuild: Only run objtool if there is at least one command
objtool: Ignore jumps to the end of the function for non-CFG arches
objtool: Allow empty alternatives
objtool: Reuse consecutive string references
objtool: Introduce objtool for arm64
klp-build: Support cross-compilation

 arch/arm64/Kconfig                            |   2 +
 arch/arm64/kernel/entry.S                     |   2 +
 arch/arm64/kernel/head.S                      |   2 +-
 arch/arm64/kernel/proton-pack.c               |  12 +-
 arch/arm64/kernel/vmlinux.lds.S               |   2 +-
 arch/x86/boot/startup/Makefile                |   2 +-
 include/asm-generic/vmlinux.lds.h             |   2 +-
 include/linux/init.h                          |   1 +
 lib/crypto/arm64/sha2-armv8.pl                |  11 +-
 scripts/Makefile.build                        |   4 +-
 scripts/Makefile.lib                          |  46 +++----
 scripts/Makefile.vmlinux_o                    |  15 +--
 scripts/livepatch/klp-build                   |  11 +-
 tools/objtool/Makefile                        |   4 +
 tools/objtool/arch/arm64/Build                |   2 +
 tools/objtool/arch/arm64/decode.c             | 116 ++++++++++++++++++
 .../arch/arm64/include/arch/cfi_regs.h        |  11 ++
 tools/objtool/arch/arm64/include/arch/elf.h   |  13 ++
 .../objtool/arch/arm64/include/arch/special.h |  21 ++++
 tools/objtool/arch/arm64/special.c            |  21 ++++
 tools/objtool/builtin-check.c                 |   5 -
 tools/objtool/check.c                         |  83 +++++++++----
 tools/objtool/elf.c                           |   9 +-
 tools/objtool/include/objtool/checksum.h      |   6 +-
 24 files changed, 321 insertions(+), 82 deletions(-)
 create mode 100644 tools/objtool/arch/arm64/Build
 create mode 100644 tools/objtool/arch/arm64/decode.c
 create mode 100644 tools/objtool/arch/arm64/include/arch/cfi_regs.h
 create mode 100644 tools/objtool/arch/arm64/include/arch/elf.h
 create mode 100644 tools/objtool/arch/arm64/include/arch/special.h
 create mode 100644 tools/objtool/arch/arm64/special.c
Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH v2 1/12 arm64: Annotate intra-function calls

In preparation for enabling objtool on arm64, annotate intra-function calls.

---
 arch/arm64/kernel/entry.S       |  2 ++
 arch/arm64/kernel/proton-pack.c | 12 +++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)
arch/arm64/kernel/entry.S +2 -0
index f8018b5c1f9a..cf808bb2abc0 100644
@@ -10,6 +10,7 @@
 #include <linux/arm-smccc.h>
 #include <linux/init.h>
 #include <linux/linkage.h>
+#include <linux/annotate.h>
 
 #include <asm/alternative.h>
 #include <asm/assembler.h>
@@ -707,6 +708,7 @@ alternative_else_nop_endif
 	 * entry onto the return stack and using a RET instruction to
 	 * enter the full-fat kernel vectors.
 	 */
+	ANNOTATE_INTRA_FUNCTION_CALL
 	bl	2f
 	b	.
 2:
arch/arm64/kernel/proton-pack.c +7 -5
index b3801f532b10..b63887a1b823 100644
@@ -24,6 +24,7 @@
 #include <linux/nospec.h>
 #include <linux/prctl.h>
 #include <linux/sched/task_stack.h>
+#include <linux/annotate.h>
 
 #include <asm/debug-monitors.h>
 #include <asm/insn.h>
@@ -245,11 +246,12 @@ static noinstr void qcom_link_stack_sanitisation(void)
 {
 	u64 tmp;
 
-	asm volatile("mov	%0, x30		\n"
-		     ".rept	16		\n"
-		     "bl	. + 4		\n"
-		     ".endr			\n"
-		     "mov	x30, %0		\n"
+	asm volatile("mov	%0, x30			\n"
+		     ".rept	16			\n"
+		     ANNOTATE_INTRA_FUNCTION_CALL "	\n"
+		     "bl	. + 4			\n"
+		     ".endr				\n"
+		     "mov	x30, %0			\n"
 		     : "=&r" (tmp));
 }
 
Signature · 1 line
2.53.0
author maintainer
#2 lore raw

PATCH v2 2/12 arm64: head: Move boot header to .head.data

In reply to Josh Poimboeuf

The arm64 boot header is mostly data. Move it to a data section to prevent objtool and other tools from trying to disassemble it. The final linked result is the same.

---
 arch/arm64/kernel/head.S          | 2 +-
 include/asm-generic/vmlinux.lds.h | 2 +-
 include/linux/init.h              | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)
arch/arm64/kernel/head.S +1 -1
index 87a822e5c4ca..0681c6e50859 100644
@@ -54,7 +54,7 @@
  * that are useful before the MMU is enabled. The allocations are described
  * in the entry routines.
  */
-	__HEAD
+	__HEADDATA
 	/*
 	 * DO NOT MODIFY. Image header expected by Linux boot-loaders.
 	 */
include/asm-generic/vmlinux.lds.h +1 -1
index 1e1580febe4b..f82f170a97eb 100644
@@ -663,7 +663,7 @@
 		__static_call_text_end = .;
 
 /* Section used for early init (in .S files) */
-#define HEAD_TEXT  KEEP(*(.head.text))
+#define HEAD_TEXT  KEEP(*(.head.data .head.text))
 
 #define HEAD_TEXT_SECTION							\
 	.head.text : AT(ADDR(.head.text) - LOAD_OFFSET) {		\
include/linux/init.h +1 -0
index 40331923b9f4..91e16f3205e2 100644
@@ -90,6 +90,7 @@
 
 /* For assembly routines */
 #define __HEAD		.section	".head.text","ax"
+#define __HEADDATA	.section	".head.data","aw"
 #define __INIT		.section	".init.text","ax"
 #define __FINIT		.previous
 
Signature · 1 line
2.53.0
author maintainer
#3 lore raw

PATCH v2 3/12 arm64: Fix EFI linking with -fdata-sections

In reply to Josh Poimboeuf

When building with func-fdata-sections, the .init.bss section gets split up into a bunch of .init.bss.<var> sections. Make sure they get linked into .init.data.

---
 arch/arm64/kernel/vmlinux.lds.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/vmlinux.lds.S +1 -1
index 2964aad0362e..f825e6a26918 100644
@@ -269,7 +269,7 @@ SECTIONS
 		INIT_CALLS
 		CON_INITCALL
 		INIT_RAM_FS
-		*(.init.altinstructions .init.bss)	/* from the EFI stub */
+		*(.init.altinstructions .init.bss .init.bss.*)	/* from the EFI stub */
 	}
 	.exit.data : {
 		EXIT_DATA
Signature · 1 line
2.53.0
author maintainer
#4 lore raw

PATCH v2 4/12 crypto: arm64: Move data to .rodata

In reply to Josh Poimboeuf

Data embedded in .text pollutes i-cache and confuses objtool and other tools that try to disassemble it. Move it to .rodata.

---
 lib/crypto/arm64/sha2-armv8.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
lib/crypto/arm64/sha2-armv8.pl +8 -3
index 35ec9ae99fe1..8cd86768bf5c 100644
@@ -237,7 +237,8 @@ $code.=<<___;
 	ldp	$E,$F,[$ctx,#4*$SZ]
 	add	$num,$inp,$num,lsl#`log(16*$SZ)/log(2)`	// end of input
 	ldp	$G,$H,[$ctx,#6*$SZ]
-	adr	$Ktbl,.LK$BITS
+	adrp	$Ktbl,.LK$BITS
+	add	$Ktbl,$Ktbl,:lo12:.LK$BITS
 	stp	$ctx,$num,[x29,#96]
 
 .Loop:
@@ -286,6 +287,7 @@ $code.=<<___;
 	ret
 .size	$func,.-$func
 
+.pushsection .rodata
 .align	6
 .type	.LK$BITS,%object
 .LK$BITS:
@@ -365,6 +367,7 @@ $code.=<<___;
 #endif
 .asciz	"SHA$BITS block transform for ARMv8, CRYPTOGAMS by <appro\@openssl.org>"
 .align	2
+.popsection
 ___
 
 if ($SZ==4) {
@@ -385,7 +388,8 @@ sha256_block_armv8:
 	add		x29,sp,#0
 
 	ld1.32		{$ABCD,$EFGH},[$ctx]
-	adr		$Ktbl,.LK256
+	adrp		$Ktbl,.LK256
+	add		$Ktbl,$Ktbl,:lo12:.LK256
 
 .Loop_hw:
 	ld1		{@MSG[0]-@MSG[3]},[$inp],#64
@@ -648,7 +652,8 @@ sha256_block_neon:
 	mov	x29, sp
 	sub	sp,sp,#16*4
 
-	adr	$Ktbl,.LK256
+	adrp	$Ktbl,.LK256
+	add	$Ktbl,$Ktbl,:lo12:.LK256
 	add	$num,$inp,$num,lsl#6	// len to point at the end of inp
 
 	ld1.8	{@X[0]},[$inp], #16
Signature · 1 line
2.53.0
author maintainer
#5 lore raw

PATCH v2 5/12 objtool: Extricate checksum calculation from validate_branch()

In reply to Josh Poimboeuf

In preparation for porting the checksum code to other arches, make its functionality independent from the CFG reverse engineering code.

Now that it's no longer called by validate_insn(), checksum_update_insn() has to manually iterate the alternatives.

---
 tools/objtool/check.c                    | 71 +++++++++++++++++-------
 tools/objtool/include/objtool/checksum.h |  6 +-
 2 files changed, 53 insertions(+), 24 deletions(-)
tools/objtool/check.c +52 -19
index 2301fb7ff3c8..d428d63b29c6 100644
@@ -2633,8 +2633,7 @@ static bool validate_branch_enabled(void)
 {
 	return opts.stackval ||
 	       opts.orc ||
-	       opts.uaccess ||
-	       opts.checksum;
+	       opts.uaccess;
 }
 
 static int decode_sections(struct objtool_file *file)
@@ -3663,6 +3662,7 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
+#ifdef BUILD_KLP
 static int checksum_debug_init(struct objtool_file *file)
 {
 	char *dup, *s;
@@ -3705,9 +3705,30 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 				 struct instruction *insn)
 {
 	struct reloc *reloc = insn_reloc(file, insn);
+	struct alternative *alt;
 	unsigned long offset;
 	struct symbol *sym;
 
+	for (alt = insn->alts; alt; alt = alt->next) {
+		struct alt_group *alt_group = alt->insn->alt_group;
+
+		checksum_update(func, insn, &alt->type, sizeof(alt->type));
+
+		if (alt_group && alt_group->orig_group) {
+			struct instruction *alt_insn;
+
+			checksum_update(func, insn, &alt_group->feature, sizeof(alt_group->feature));
+
+			for (alt_insn = alt->insn; alt_insn; alt_insn = next_insn_same_sec(file, alt_insn)) {
+				checksum_update_insn(file, func, alt_insn);
+				if (alt_insn == alt_group->last_insn)
+					break;
+			}
+		} else {
+			checksum_update(func, insn, &alt->insn->offset, sizeof(alt->insn->offset));
+		}
+	}
+
 	if (insn->fake)
 		return;
 
@@ -3716,9 +3737,11 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	if (!reloc) {
 		struct symbol *call_dest = insn_call_dest(insn);
 
-		if (call_dest)
+		if (call_dest) {
+			/* intra-TU call without reloc */
 			checksum_update(func, insn, call_dest->demangled_name,
 					strlen(call_dest->demangled_name));
+		}
 		return;
 	}
 
@@ -3745,6 +3768,29 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func,
 	checksum_update(func, insn, &offset, sizeof(offset));
 }
 
+static int calculate_checksums(struct objtool_file *file)
+{
+	struct instruction *insn;
+	struct symbol *func;
+
+	if (checksum_debug_init(file))
+		return -1;
+
+	for_each_sym(file->elf, func) {
+		if (!is_func_sym(func))
+			continue;
+
+		checksum_init(func);
+
+		func_for_each_insn(file, func, insn)
+			checksum_update_insn(file, func, insn);
+
+		checksum_finish(func);
+	}
+	return 0;
+}
+#endif /* BUILD_KLP */
+
 static int validate_branch(struct objtool_file *file, struct symbol *func,
 			   struct instruction *insn, struct insn_state state);
 static int do_validate_branch(struct objtool_file *file, struct symbol *func,
@@ -4026,9 +4072,6 @@ static int do_validate_branch(struct objtool_file *file, struct symbol *func,
 		insn->trace = 0;
 		next_insn = next_insn_to_validate(file, insn);
 
-		if (opts.checksum && func && insn->sec)
-			checksum_update_insn(file, func, insn);
-
 		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
 			/* Ignore KCFI type preambles, which always fall through */
 			if (is_prefix_func(func))
@@ -4094,9 +4137,6 @@ static int validate_unwind_hint(struct objtool_file *file,
 		struct symbol *func = insn_func(insn);
 		int ret;
 
-		if (opts.checksum)
-			checksum_init(func);
-
 		ret = validate_branch(file, func, insn, *state);
 		if (ret)
 			BT_INSN(insn, "<=== (hint)");
@@ -4539,9 +4579,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 
 	func = insn_func(insn);
 
-	if (opts.checksum)
-		checksum_init(func);
-
 	if (opts.trace && !fnmatch(opts.trace, sym->name, 0)) {
 		trace_enable();
 		TRACE("%s: validation begin\n", sym->name);
@@ -4554,9 +4591,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	TRACE("%s: validation %s\n\n", sym->name, ret ? "failed" : "end");
 	trace_disable();
 
-	if (opts.checksum)
-		checksum_finish(func);
-
 	return ret;
 }
 
@@ -5011,10 +5045,6 @@ int check(struct objtool_file *file)
 	cfi_hash_add(&init_cfi);
 	cfi_hash_add(&func_cfi);
 
-	ret = checksum_debug_init(file);
-	if (ret)
-		goto out;
-
 	ret = decode_sections(file);
 	if (ret)
 		goto out;
@@ -5105,6 +5135,9 @@ int check(struct objtool_file *file)
 		warnings += check_abs_references(file);
 
 	if (opts.checksum) {
+		ret = calculate_checksums(file);
+		if (ret)
+			goto out;
 		ret = create_sym_checksum_section(file);
 		if (ret)
 			goto out;
tools/objtool/include/objtool/checksum.h +1 -5
index 7fe21608722a..36a17688c716 100644
@@ -32,11 +32,7 @@ static inline void checksum_finish(struct symbol *func)
 
 #else /* !BUILD_KLP */
 
-static inline void checksum_init(struct symbol *func) {}
-static inline void checksum_update(struct symbol *func,
-				   struct instruction *insn,
-				   const void *data, size_t size) {}
-static inline void checksum_finish(struct symbol *func) {}
+static inline int calculate_checksums(struct objtool_file *file) { return -ENOSYS; }
 
 #endif /* !BUILD_KLP */
 
Signature · 1 line
2.53.0
author maintainer
#6 lore raw

PATCH v2 6/12 objtool: Allow setting --mnop without --mcount

In reply to Josh Poimboeuf

Instead of returning an error for --mnop without --mcount, just silently ignore it. This will help simplify kbuild's handling of objtool args.

---
 tools/objtool/builtin-check.c | 5 -----
 1 file changed, 5 deletions(-)
tools/objtool/builtin-check.c +0 -5
index b780df513715..f528a58aca74 100644
@@ -145,11 +145,6 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])
 
 static bool opts_valid(void)
 {
-	if (opts.mnop && !opts.mcount) {
-		ERROR("--mnop requires --mcount");
-		return false;
-	}
-
 	if (opts.noinstr && !opts.link) {
 		ERROR("--noinstr requires --link");
 		return false;
Signature · 1 line
2.53.0
author maintainer
#7 lore raw

PATCH v2 7/12 kbuild: Only run objtool if there is at least one command

In reply to Josh Poimboeuf

Split the objtool args into commands and options, such that if no commands have been enabled, objtool doesn't run.

This is in preparation in enabling objtool and klp-build for arm64.

---
 arch/x86/boot/startup/Makefile |  2 +-
 scripts/Makefile.build         |  4 +--
 scripts/Makefile.lib           | 46 ++++++++++++++++++----------------
 scripts/Makefile.vmlinux_o     | 15 ++++-------
 4 files changed, 33 insertions(+), 34 deletions(-)
arch/x86/boot/startup/Makefile +1 -1
index 5e499cfb29b5..a08297829fc6 100644
@@ -36,7 +36,7 @@ $(patsubst %.o,$(obj)/%.o,$(lib-y)): OBJECT_FILES_NON_STANDARD := y
 # relocations, even if other objtool actions are being deferred.
 #
 $(pi-objs): objtool-enabled	= 1
-$(pi-objs): objtool-args	= $(if $(delay-objtool),--dry-run,$(objtool-args-y)) --noabs
+$(pi-objs): objtool-args	= $(if $(delay-objtool),--dry-run,$(objtool-cmds-y) $(objtool-opts-y)) --noabs
 
 #
 # Confine the startup code by prefixing all symbols with __pi_ (for position
scripts/Makefile.build +2 -2
index 3652b85be545..8a1bdfdb2fdb 100644
@@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
 
 ifdef CONFIG_OBJTOOL
-$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
+$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))
 endif
 
 ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
@@ -501,7 +501,7 @@ define rule_ld_multi_m
 	$(call cmd,gen_objtooldep)
 endef
 
-$(multi-obj-m): private objtool-enabled := $(delay-objtool)
+$(multi-obj-m): private objtool-enabled := $(if $(objtool-cmds-y),$(delay-objtool))
 $(multi-obj-m): private part-of-module := y
 $(multi-obj-m): %.o: %.mod FORCE
 	$(call if_changed_rule,ld_multi_m)
scripts/Makefile.lib +25 -21

59 lines not on this page · raw message

scripts/Makefile.vmlinux_o +5 -10
index 527352c222ff..09af33203bd8 100644
@@ -36,18 +36,13 @@ endif
 # For !delay-objtool + CONFIG_NOINSTR_VALIDATION, it runs on both translation
 # units and vmlinux.o, with the latter only used for noinstr/unret validation.
 
-objtool-enabled := $(or $(delay-objtool),$(CONFIG_NOINSTR_VALIDATION))
-
-ifeq ($(delay-objtool),y)
-vmlinux-objtool-args-y					+= $(objtool-args-y)
-else
-vmlinux-objtool-args-$(CONFIG_OBJTOOL_WERROR)		+= --werror
+ifneq ($(delay-objtool),y)
+objtool-cmds-y					 =
+objtool-opts-y					+= --link
 endif
 
-vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
-							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
-
-objtool-args = $(vmlinux-objtool-args-y) --link
+objtool-cmds-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
+						   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
 
 # Link of vmlinux.o used for section mismatch analysis
 # ---------------------------------------------------------------------------
Signature · 1 line
2.53.0
maintainer Reviewed-by on v2 7/12
#8 lore raw

Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command

Quoted 15 lines · Josh Poimboeuf

Split the objtool args into commands and options, such that if no commands have been enabled, objtool doesn't run.

This is in preparation in enabling objtool and klp-build for arm64.

---
 arch/x86/boot/startup/Makefile |  2 +-
 scripts/Makefile.build         |  4 +--
 scripts/Makefile.lib           | 46 ++++++++++++++++++----------------
 scripts/Makefile.vmlinux_o     | 15 ++++-------
 4 files changed, 33 insertions(+), 34 deletions(-)

[...]

scripts/Makefile.build endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT · line 280

Quoted 10 lines
scripts/Makefile.build +1 -1
index 3652b85be545..8a1bdfdb2fdb 100644
@@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
 
 ifdef CONFIG_OBJTOOL
-$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
+$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))

Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if $(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be left-out:

$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))

scripts/Makefile.build define rule_ld_multi_m · line 504

Quoted 9 lines
scripts/Makefile.build +1 -1
 endif
 
 ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
@@ -501,7 +501,7 @@ define rule_ld_multi_m
 	$(call cmd,gen_objtooldep)
 endef
 
-$(multi-obj-m): private objtool-enabled := $(delay-objtool)
+$(multi-obj-m): private objtool-enabled := $(if $(objtool-cmds-y),$(delay-objtool))

Could be changed to $(and), too; but personally I think the $(if) is easier to parse at once.

Signature · 1 line
Nicolas
author maintainer
#9 lore raw

Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command

Quoted 32 lines · Nicolas Schier
Quoted 15 lines · Josh Poimboeuf

Split the objtool args into commands and options, such that if no commands have been enabled, objtool doesn't run.

This is in preparation in enabling objtool and klp-build for arm64.

---
 arch/x86/boot/startup/Makefile |  2 +-
 scripts/Makefile.build         |  4 +--
 scripts/Makefile.lib           | 46 ++++++++++++++++++----------------
 scripts/Makefile.vmlinux_o     | 15 ++++-------
 4 files changed, 33 insertions(+), 34 deletions(-)

[...]

scripts/Makefile.build endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT · line 280

Quoted 10 lines
scripts/Makefile.build +1 -1
index 3652b85be545..8a1bdfdb2fdb 100644
@@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
 
 ifdef CONFIG_OBJTOOL
-$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
+$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))

Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if $(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be left-out:

Quoted 1 line

$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))

I believe that would break the !delay-objtool case. The logic needs to be something like:

if (is-standard-object && objtool-cmds-y) {
	if (delay-objtool) {
		// for delay-objtool, only enable objtool for single-object modules
		$(is-single-obj-m)
	} else {
		// for !delay-objtool, always enable objtool
		y
	}
}

so maybe something like this?

$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y))

Signature · 1 line
Josh
maintainer
#10 lore raw

Re: [PATCH v2 07/12] kbuild: Only run objtool if there is at least one command

Quoted 48 lines · Josh Poimboeuf
Quoted 32 lines · Nicolas Schier
Quoted 15 lines · Josh Poimboeuf

Split the objtool args into commands and options, such that if no commands have been enabled, objtool doesn't run.

This is in preparation in enabling objtool and klp-build for arm64.

---
 arch/x86/boot/startup/Makefile |  2 +-
 scripts/Makefile.build         |  4 +--
 scripts/Makefile.lib           | 46 ++++++++++++++++++----------------
 scripts/Makefile.vmlinux_o     | 15 ++++-------
 4 files changed, 33 insertions(+), 34 deletions(-)

[...]

scripts/Makefile.build endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT · line 280

Quoted 10 lines
scripts/Makefile.build +1 -1
index 3652b85be545..8a1bdfdb2fdb 100644
@@ -277,7 +277,7 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(target-stem).o)$(OBJECT_FILES_NON_STANDARD)n),$(is-kernel-object))
 
 ifdef CONFIG_OBJTOOL
-$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
+$(obj)/%.o: private objtool-enabled = $(if $(is-standard-object),$(if $(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y)))

Please use $(and a,b,c) instead of multiple nested $(if $(a),$(if $(b),$(c)); as the last variable (is-single-obj-m) is 'y' or empty, the final 'y' can be left-out:

Quoted 1 line

$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(delay-objtool),$(is-single-obj-m))

I believe that would break the !delay-objtool case. The logic needs to be something like:

if (is-standard-object && objtool-cmds-y) {
	if (delay-objtool) {
		// for delay-objtool, only enable objtool for single-object modules
		$(is-single-obj-m)
	} else {
		// for !delay-objtool, always enable objtool
		y
	}
}
Quoted 3 lines

so maybe something like this?

$(obj)/%.o: private objtool-enabled = $(and $(is-standard-object),$(objtool-cmds-y),$(if $(delay-objtool),$(is-single-obj-m),y))

sorry for the delay! Yes, I overlooked the !delay-objtool. That line looks good to me, thanks!

Signature · 1 line
Nicolas
author maintainer
#11 lore raw

PATCH v2 8/12 objtool: Ignore jumps to the end of the function for non-CFG arches

In reply to Josh Poimboeuf

Sometimes Clang arm64 code jumps to the end of the function for UB. No need to make that an error, arm64 doesn't reverse engineer the CFG anyway.

---
 tools/objtool/check.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
tools/objtool/check.c +5 -3
index d428d63b29c6..6e33a29bd3ed 100644
@@ -1630,10 +1630,12 @@ static int add_jump_destinations(struct objtool_file *file)
 			/*
 			 * GCOV/KCOV dead code can jump to the end of
 			 * the function/section.
+			 *
+			 * Clang on arm64 also does this sometimes for
+			 * undefined behavior.
 			 */
-			if (file->ignore_unreachables && func &&
-			    dest_sec == insn->sec &&
-			    dest_off == func->offset + func->len)
+			if ((file->ignore_unreachables || (!opts.stackval && !opts.orc)) &&
+			    func && dest_sec == insn->sec && dest_off == func->offset + func->len)
 				continue;
 
 			ERROR_INSN(insn, "can't find jump dest instruction at %s",
Signature · 1 line
2.53.0
author maintainer
#12 lore raw

PATCH v2 9/12 objtool: Allow empty alternatives

In reply to Josh Poimboeuf

arm64 can have empty alternatives, which are effectively no-ops. Ignore them.

---
 tools/objtool/check.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
tools/objtool/check.c +1 -3
index 6e33a29bd3ed..7297fb87f876 100644
@@ -1994,10 +1994,8 @@ static int add_special_section_alts(struct objtool_file *file)
 		}
 
 		if (special_alt->group) {
-			if (!special_alt->orig_len) {
-				ERROR_INSN(orig_insn, "empty alternative entry");
+			if (!special_alt->orig_len)
 				continue;
-			}
 
 			if (handle_group_alt(file, special_alt, orig_insn, &new_insn))
 				return -1;
Signature · 1 line
2.53.0
author maintainer
#13 lore raw

PATCH v2 10/12 objtool: Reuse consecutive string references

In reply to Josh Poimboeuf

For duplicate strings, elf_add_string() just blindly adds duplicates.

That can be a problem for arm64 which often uses two consecutive instructions (and corresponding relocations) to put an address into a register, like:

  d8:   90000001        adrp    x1, 0 <meminfo_proc_show>       d8: R_AARCH64_ADR_PREL_PG_HI21  .rodata.meminfo_proc_show.str1.8
  dc:   91000021        add     x1, x1, #0x0    dc: R_AARCH64_ADD_ABS_LO12_NC   .rodata.meminfo_proc_show.str1.8

Referencing two different string addresses in the adrp+add pair can result in a corrupt string addresses. Detect such consecutive reuses and force them to use the same string.

---
 tools/objtool/elf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
tools/objtool/elf.c +8 -1
index b911abe3a15e..5ccae2c937dc 100644
@@ -1374,6 +1374,8 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
 
 unsigned int elf_add_string(struct elf *elf, struct section *strtab, const char *str)
 {
+	static unsigned int last_off;
+	static const char *last_str;
 	unsigned int offset;
 
 	if (!strtab)
@@ -1382,17 +1384,22 @@ unsigned int elf_add_string(struct elf *elf, struct section *strtab, const char
 		ERROR("can't find .strtab section");
 		return -1;
 	}
-
 	if (!strtab->sh.sh_addralign) {
 		ERROR("'%s': invalid sh_addralign", strtab->name);
 		return -1;
 	}
 
+	if (last_str && !strcmp(last_str, str))
+		return last_off;
+
 	offset = ALIGN(sec_size(strtab), strtab->sh.sh_addralign);
 
 	if (!elf_add_data(elf, strtab, str, strlen(str) + 1))
 		return -1;
 
+	last_str = str;
+	last_off = offset;
+
 	return offset;
 }
 
Signature · 1 line
2.53.0
author maintainer
#14 lore raw

PATCH v2 11/12 objtool: Introduce objtool for arm64

In reply to Josh Poimboeuf

Add basic support for arm64 on objtool. Only --checksum is currently supported.

---
 arch/arm64/Kconfig                            |   2 +
 tools/objtool/Makefile                        |   4 +
 tools/objtool/arch/arm64/Build                |   2 +
 tools/objtool/arch/arm64/decode.c             | 116 ++++++++++++++++++
 .../arch/arm64/include/arch/cfi_regs.h        |  11 ++
 tools/objtool/arch/arm64/include/arch/elf.h   |  13 ++
 .../objtool/arch/arm64/include/arch/special.h |  21 ++++
 tools/objtool/arch/arm64/special.c            |  21 ++++
 8 files changed, 190 insertions(+)
 create mode 100644 tools/objtool/arch/arm64/Build
 create mode 100644 tools/objtool/arch/arm64/decode.c
 create mode 100644 tools/objtool/arch/arm64/include/arch/cfi_regs.h
 create mode 100644 tools/objtool/arch/arm64/include/arch/elf.h
 create mode 100644 tools/objtool/arch/arm64/include/arch/special.h
 create mode 100644 tools/objtool/arch/arm64/special.c
arch/arm64/Kconfig +2 -0
index 38dba5f7e4d2..354aa80c5b4b 100644
@@ -236,9 +236,11 @@ config ARM64
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
 	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_TIME_ACCOUNTING
+	select HAVE_KLP_BUILD
 	select HAVE_LIVEPATCH
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI
+	select HAVE_OBJTOOL
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
 	select HAVE_PERF_REGS
tools/objtool/Makefile +4 -0
index b71d1886022e..66b04172b79a 100644
@@ -11,6 +11,10 @@ ifeq ($(SRCARCH),loongarch)
 	BUILD_ORC	   := y
 endif
 
+ifeq ($(SRCARCH),arm64)
+	ARCH_HAS_KLP := y
+endif
+
 ifeq ($(ARCH_HAS_KLP),y)
 	HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
 		      $(HOSTCC) $(HOSTCFLAGS) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
tools/objtool/arch/arm64/Build new file +2 -0
new file mode 100644
index 000000000000..d24d5636a5b8
@@ -0,0 +1,2 @@
+objtool-y += decode.o
+objtool-y += special.o
tools/objtool/arch/arm64/decode.c new file +116 -0

116 lines not on this page · raw message

tools/objtool/arch/arm64/include/arch/cfi_regs.h new file +11 -0
new file mode 100644
index 000000000000..49c81cbb6646
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_CFI_REGS_H
+#define _OBJTOOL_ARCH_CFI_REGS_H
+
+/* These aren't actually used for arm64 */
+#define CFI_BP 0
+#define CFI_SP 0
+#define CFI_RA 0
+#define CFI_NUM_REGS 2
+
+#endif /* _OBJTOOL_ARCH_CFI_REGS_H */
tools/objtool/arch/arm64/include/arch/elf.h new file +13 -0
new file mode 100644
index 000000000000..80a1bc479930
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_ELF_H
+#define _OBJTOOL_ARCH_ELF_H
+
+#define R_NONE		R_AARCH64_NONE
+#define R_ABS64		R_AARCH64_ABS64
+#define R_ABS32		R_AARCH64_ABS32
+#define R_DATA32	R_AARCH64_PREL32
+#define R_DATA64	R_AARCH64_PREL64
+#define R_TEXT32	R_AARCH64_PREL32
+#define R_TEXT64	R_AARCH64_PREL64
+
+#endif /* _OBJTOOL_ARCH_ELF_H */
tools/objtool/arch/arm64/include/arch/special.h new file +21 -0
new file mode 100644
index 000000000000..8ae804a83ea4
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _OBJTOOL_ARCH_SPECIAL_H
+#define _OBJTOOL_ARCH_SPECIAL_H
+
+#define EX_ENTRY_SIZE 12
+#define EX_ORIG_OFFSET 0
+#define EX_NEW_OFFSET 4
+
+#define JUMP_ENTRY_SIZE 16
+#define JUMP_ORIG_OFFSET 0
+#define JUMP_NEW_OFFSET 4
+#define JUMP_KEY_OFFSET 8
+
+#define ALT_ENTRY_SIZE 12
+#define ALT_ORIG_OFFSET 0
+#define ALT_NEW_OFFSET 4
+#define ALT_FEATURE_OFFSET 8
+#define ALT_ORIG_LEN_OFFSET 10
+#define ALT_NEW_LEN_OFFSET 11
+
+#endif /* _OBJTOOL_ARCH_SPECIAL_H */
tools/objtool/arch/arm64/special.c new file +21 -0
new file mode 100644
index 000000000000..3c2b83d94939
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <objtool/special.h>
+
+bool arch_support_alt_relocation(struct special_alt *special_alt,
+				 struct instruction *insn,
+				 struct reloc *reloc)
+{
+	return false;
+}
+
+struct reloc *arch_find_switch_table(struct objtool_file *file,
+				     struct instruction *insn,
+				     unsigned long *table_size)
+{
+	return NULL;
+}
+
+const char *arch_cpu_feature_name(int feature_number)
+{
+	return NULL;
+}
Signature · 1 line
2.53.0
author maintainer
#15 lore raw

PATCH v2 12/12 klp-build: Support cross-compilation

In reply to Josh Poimboeuf

Add support for cross-compilation. The user must export ARCH, and either CROSS_COMPILE or LLVM as needed.

---
 scripts/livepatch/klp-build | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
scripts/livepatch/klp-build +10 -1
index 809e198a561d..b6c057e2120f 100755
@@ -404,6 +404,14 @@ validate_patches() {
 	revert_patches
 }
 
+cross_compile_init() {
+	if [[ -v LLVM ]]; then
+		OBJCOPY=llvm-objcopy
+	else
+		OBJCOPY="${CROSS_COMPILE:-}objcopy"
+	fi
+}
+
 do_init() {
 	# We're not yet smart enough to handle anything other than in-tree
 	# builds in pwd.
@@ -420,6 +428,7 @@ do_init() {
 	validate_config
 	set_module_name
 	set_kernelversion
+	cross_compile_init
 }
 
 # Refresh the patch hunk headers, specifically the line numbers and counts.
@@ -783,7 +792,7 @@ build_patch_module() {
 	cp -f "$kmod_file" "$kmod_file.orig"
 
 	# Work around issue where slight .config change makes corrupt BTF
-	objcopy --remove-section=.BTF "$kmod_file"
+	"$OBJCOPY" --remove-section=.BTF "$kmod_file"
 
 	# Fix (and work around) linker wreckage for klp syms / relocs
 	"$SRC/tools/objtool/objtool" klp post-link "$kmod_file" || die "objtool klp post-link failed"
Signature · 1 line
2.53.0
author maintainer
#16 lore raw
In reply to Josh Poimboeuf
Quoted 10 lines · Josh Poimboeuf

v2:
- patches 1-2 were merged, rebase on tip/master
- improve commit message for "objtool: Extricate checksum calculation from validate_branch()"
- add review tags

v1: https://lore.kernel.org/[email protected]

Port objtool and the klp-build tooling (for building livepatch modules) to arm64.

Quoted 7 lines

Note this doesn't bring all the objtool bells and whistles to arm64, nor any of the CFG reverse engineering. This only adds the bare minimum needed for 'objtool --checksum'.

And note that objtool still doesn't get enabled at all for normal arm64 kernel builds, so this doesn't affect any users other than those running klp-build directly.

The Sashiko AI thing found some bugs, so it might be wise to hold off on any review or testing of this until I get v3 out.

Signature · 1 line
Josh