From 750229e94bf773ec6f13fce1eac25da80d1b31ec Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 21 Feb 2024 21:17:27 -0800 Subject: [PATCH] create-diff-object: Add support for CONFIG_X86_KERNEL_IBT With IBT enabled, objtool runs on the final linked vmlinux.o object instead of the individual translation units, creating the __pfx symbols at the end. But create-diff-object still runs on the individual .o objects, in which case the __pfx symbols may be missing. Manually detect function padding for that case. With this change, it should be fine [*] to patch a kernel with CONFIG_X86_KERNEL_IBT enabled. [*] Unless your patch adds an indirect call to an existing function which doesn't have any other indirect callers, in which case the callee might have been sealed, which will trigger a "Missing ENDBR" warning/panic. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 57 +++++++++++++++++++++++++++++++ kpatch-build/kpatch-build | 3 ++ test/unit/Makefile.include | 6 ++-- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 25710e921..eb1e28a54 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -228,6 +228,61 @@ static struct rela *toc_rela(const struct rela *rela) (unsigned int)rela->addend); } +static unsigned int nop_bytes_before_function(struct kpatch_elf *kelf, struct symbol *sym) +{ + switch (kelf->arch) { + case X86_64: + { + unsigned char *insn = sym->sec->data->d_buf; + unsigned int size = 0, i; + + for (i = 0; i < sym->sym.st_value; i++, insn++) { + if (*insn != 0x90) + ERROR("unexpected insn in function padding area"); + size++; + } + return size; + } + default: + break; + } + + return 0; +} + +static unsigned int function_padding_size(struct kpatch_elf *kelf, struct symbol *sym) +{ + static unsigned int size = (unsigned int)-1; + char *str; + + if (size == (unsigned int)-1) { + str = getenv("CONFIG_FUNCTION_PADDING_BYTES"); + if (str) + size = atoi(str); + else if (!getenv("KPATCH_UNIT_TEST")) + size = 0; + else { + /* + * This is running in the context of a unit test. We + * could in theory require unit tests to set + * CONFIG_FUNCTION_PADDING_BYTES in CDO_ENV and/or in + * the test-specific .env file. However that might be + * more trouble than it's worth (lots of .env file + * fiddling). + * + * For now, just find the padding dynamically, and + * validate the size and that all other bundled + * functions have the same padding. + */ + size = nop_bytes_before_function(kelf, sym); + if (size != 0 && size != 16) + ERROR("unexpected bundled function offset in unit test: %u", size); + } + } + + return size; +} + /* * When compiling with -ffunction-sections and -fdata-sections, almost every * symbol gets its own dedicated section. We call such symbols "bundled" @@ -244,6 +299,8 @@ static void kpatch_bundle_symbols(struct kpatch_elf *kelf) expected_offset = 16; else if (is_gcc6_localentry_bundled_sym(kelf, sym)) expected_offset = 8; + else if (sym->type == STT_FUNC && !sym->parent) + expected_offset = function_padding_size(kelf, sym); else expected_offset = 0; diff --git a/kpatch-build/kpatch-build b/kpatch-build/kpatch-build index 6bdad3bb3..26ae78a02 100755 --- a/kpatch-build/kpatch-build +++ b/kpatch-build/kpatch-build @@ -1114,6 +1114,9 @@ trace_off "reading .config" source "$CONFIGFILE" trace_on +# for create-diff-object +export CONFIG_FUNCTION_PADDING_BYTES + [[ "$DISTRO" = openEuler ]] && [[ -z "$CONFIG_LIVEPATCH_PER_TASK_CONSISTENCY" ]] && \ die "openEuler kernel doesn't have 'CONFIG_LIVEPATCH_PER_TASK_CONSISTENCY' enabled" diff --git a/test/unit/Makefile.include b/test/unit/Makefile.include index e9717430e..3816541dc 100644 --- a/test/unit/Makefile.include +++ b/test/unit/Makefile.include @@ -57,7 +57,8 @@ clean: @echo "BUILD $(TNAME)" $(call check_all,$(TNAME).$(EXT_ORIG)) $(call check_all,$(TNAME).$(EXT_PATCHED)) - $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_PATCHED) \ + KPATCH_UNIT_TEST=1 $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) \ + $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_PATCHED) \ vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \ test_$(TNAME) $@ $(MUTE_PASS) @@ -65,7 +66,8 @@ clean: @echo "BUILD $(TNAME)-FAIL" $(call check_all,$(TNAME).$(EXT_ORIG)) $(call check_all,$(TNAME).$(EXT_FAIL)) - ! $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_FAIL) \ + ! KPATCH_UNIT_TEST=1 $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) \ + $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_FAIL) \ vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \ test_$(TNAME) $@ $(MUTE_FAIL) # Expecting to fail, thus create output file manually so we won't rerun the