Skip to content

Commit

Permalink
create-diff-object: Add support for CONFIG_X86_KERNEL_IBT
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
jpoimboe committed Mar 19, 2024
1 parent 4077d87 commit e53a551
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 5 deletions.
63 changes: 60 additions & 3 deletions kpatch-build/create-diff-object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;

Expand Down Expand Up @@ -4074,12 +4131,12 @@ int main(int argc, char *argv[])
kpatch_check_program_headers(kelf_orig->elf);
kpatch_check_program_headers(kelf_patched->elf);

kpatch_bundle_symbols(kelf_orig);
kpatch_bundle_symbols(kelf_patched);

kpatch_detect_child_functions(kelf_orig);
kpatch_detect_child_functions(kelf_patched);

kpatch_bundle_symbols(kelf_orig);
kpatch_bundle_symbols(kelf_patched);

lookup = lookup_open(parent_symtab, parent_name, mod_symvers, kelf_orig);

kpatch_mark_grouped_sections(kelf_patched);
Expand Down
3 changes: 3 additions & 0 deletions kpatch-build/kpatch-build
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
6 changes: 4 additions & 2 deletions test/unit/Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,17 @@ 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)

%.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_FAIL) %.$(EXT_SYMTAB) $(CDO)
@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
Expand Down

0 comments on commit e53a551

Please sign in to comment.