From 0c5a1e77532ef0047d411d8782188a1ec74896d6 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 13:54:24 -0700 Subject: [PATCH 01/17] kpatch-build: make xtrace output less verbose With '--debug', most of the xtrace output shows the reading of the .config and Module.symvers files, which isn't very useful and floods the rest of the xtrace output. Temporarily disable xtrace before reading the files. Signed-off-by: Josh Poimboeuf --- kpatch-build/kpatch-build | 54 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/kpatch-build/kpatch-build b/kpatch-build/kpatch-build index b1f03e3..f8745d4 100755 --- a/kpatch-build/kpatch-build +++ b/kpatch-build/kpatch-build @@ -97,6 +97,18 @@ logger() { fi } +trace_on() { + if [[ $DEBUG -eq 1 ]] || [[ $DEBUG -ge 3 ]]; then + set -o xtrace + fi +} + +trace_off() { + if [[ $DEBUG -eq 1 ]] || [[ $DEBUG -ge 3 ]]; then + set +o xtrace + fi +} + save_env() { export -p | grep -wv -e 'OLDPWD=' -e 'PWD=' > "$ENVFILE" } @@ -619,9 +631,7 @@ if [[ ${#PATCH_LIST[@]} -eq 0 ]]; then exit 1 fi -if [[ $DEBUG -eq 1 ]] || [[ $DEBUG -ge 3 ]]; then - set -o xtrace -fi +trace_on if [[ -n "$SRCRPM" ]]; then if [[ -n "$ARCHVERSION" ]]; then @@ -822,9 +832,11 @@ fi # kernel option checking +trace_off "reading .config" # Don't check external file. # shellcheck disable=SC1090 source "$CONFIGFILE" +trace_on [[ -z "$CONFIG_DEBUG_INFO" ]] && die "kernel doesn't have 'CONFIG_DEBUG_INFO' enabled" @@ -971,26 +983,28 @@ fi grep -q vmlinux "$KERNEL_SRCDIR/Module.symvers" || die "truncated $KERNEL_SRCDIR/Module.symvers file" if [[ -n "$CONFIG_MODVERSIONS" ]]; then - while read -ra sym_line; do - if [[ ${#sym_line[@]} -lt 4 ]]; then - die "Malformed ${TEMPDIR}/Module.symvers file" - fi + trace_off "reading Module.symvers" + while read -ra sym_line; do + if [[ ${#sym_line[@]} -lt 4 ]]; then + die "Malformed ${TEMPDIR}/Module.symvers file" + fi - sym=${sym_line[1]} + sym=${sym_line[1]} - read -ra patched_sym_line <<< "$(grep "\s$sym\s" "$BUILDDIR/Module.symvers")" - if [[ ${#patched_sym_line[@]} -lt 4 ]]; then - die "Malformed symbol entry for ${sym} in ${BUILDDIR}/Module.symvers file" - fi + read -ra patched_sym_line <<< "$(grep "\s$sym\s" "$BUILDDIR/Module.symvers")" + if [[ ${#patched_sym_line[@]} -lt 4 ]]; then + die "Malformed symbol entry for ${sym} in ${BUILDDIR}/Module.symvers file" + fi - # Assume that both original and patched symvers have the same format. - # In both cases, the symbol should have the same CRC, belong to the same - # Module/Namespace and have the same export type. - if [[ ${#sym_line[@]} -ne ${#patched_sym_line[@]} || \ - "${sym_line[*]}" != "${patched_sym_line[*]}" ]]; then - warn "Version disagreement for symbol ${sym}" - fi - done < "${TEMPDIR}/Module.symvers" + # Assume that both original and patched symvers have the same format. + # In both cases, the symbol should have the same CRC, belong to the same + # Module/Namespace and have the same export type. + if [[ ${#sym_line[@]} -ne ${#patched_sym_line[@]} || \ + "${sym_line[*]}" != "${patched_sym_line[*]}" ]]; then + warn "Version disagreement for symbol ${sym}" + fi + done < "${TEMPDIR}/Module.symvers" + trace_on fi # Read as words, no quotes. From 3b634568171f89a9a1c2d68e4d1a208d33d8cb4e Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:57:12 -0700 Subject: [PATCH 02/17] kpatch-elf: convert functions to static These functions are only called locally, convert them to static. Signed-off-by: Josh Poimboeuf --- kpatch-build/kpatch-elf.c | 6 +++--- kpatch-build/kpatch-elf.h | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index c6ef6f1..1de8613 100644 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -164,7 +164,7 @@ int offset_of_string(struct list_head *list, char *name) return index; } -void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec) +static void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec) { int index = 0, skip = 0; struct rela *rela; @@ -223,7 +223,7 @@ void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec) } } -void kpatch_create_section_list(struct kpatch_elf *kelf) +static void kpatch_create_section_list(struct kpatch_elf *kelf) { Elf_Scn *scn = NULL; struct section *sec; @@ -277,7 +277,7 @@ void kpatch_create_section_list(struct kpatch_elf *kelf) ERROR("expected NULL"); } -void kpatch_create_symbol_list(struct kpatch_elf *kelf) +static void kpatch_create_symbol_list(struct kpatch_elf *kelf) { struct section *symtab; struct symbol *sym; diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h index abc6efd..fbaa703 100644 --- a/kpatch-build/kpatch-elf.h +++ b/kpatch-build/kpatch-elf.h @@ -160,9 +160,6 @@ int offset_of_string(struct list_head *list, char *name); /************* * Functions * **********/ -void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec); -void kpatch_create_section_list(struct kpatch_elf *kelf); -void kpatch_create_symbol_list(struct kpatch_elf *kelf); struct kpatch_elf *kpatch_elf_open(const char *name); void kpatch_dump_kelf(struct kpatch_elf *kelf); From c24d135f403cdaf71158b185705c4214fb9d6e3e Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:02:58 -0700 Subject: [PATCH 03/17] create-diff-object: rename "sec" -> "relasec" for rela sections Several functions expect to take a ".rela" section as an argument. Make such cases more clear by renaming "sec" -> "relasec". Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 239 +++++++++++++++--------------- kpatch-build/create-klp-module.c | 46 +++--- kpatch-build/kpatch-elf.c | 31 ++-- 3 files changed, 159 insertions(+), 157 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 826741d..903b92c 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -506,7 +506,7 @@ static bool rela_equal(struct rela *rela1, struct rela *rela2) return !kpatch_mangled_strcmp(rela_toc1->sym->name, rela_toc2->sym->name); } -static void kpatch_compare_correlated_rela_section(struct section *sec) +static void kpatch_compare_correlated_rela_section(struct section *relasec) { struct rela *rela1, *rela2 = NULL; @@ -514,22 +514,22 @@ static void kpatch_compare_correlated_rela_section(struct section *sec) * On ppc64le, don't compare the .rela.toc section. The .toc and * .rela.toc sections are included as standard elements. */ - if (!strcmp(sec->name, ".rela.toc")) { - sec->status = SAME; + if (!strcmp(relasec->name, ".rela.toc")) { + relasec->status = SAME; return; } - rela2 = list_entry(sec->twin->relas.next, struct rela, list); - list_for_each_entry(rela1, &sec->relas, list) { + rela2 = list_entry(relasec->twin->relas.next, struct rela, list); + list_for_each_entry(rela1, &relasec->relas, list) { if (rela_equal(rela1, rela2)) { rela2 = list_entry(rela2->list.next, struct rela, list); continue; } - sec->status = CHANGED; + relasec->status = CHANGED; return; } - sec->status = SAME; + relasec->status = SAME; } static void kpatch_compare_correlated_nonrela_section(struct section *sec) @@ -1138,13 +1138,13 @@ static char *kpatch_section_function_name(struct section *sec) return sec->sym ? sec->sym->name : sec->name; } -static struct symbol *kpatch_find_uncorrelated_rela(struct section *rela_sec, +static struct symbol *kpatch_find_uncorrelated_rela(struct section *relasec, struct symbol *sym) { struct rela *rela, *rela_toc; /* find the patched object's corresponding variable */ - list_for_each_entry(rela, &rela_sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { struct symbol *patched_sym; rela_toc = toc_rela(rela); @@ -1198,34 +1198,34 @@ static struct symbol *kpatch_find_static_twin_in_children(struct symbol *parent, * in the base object, find a corresponding usage of a similarly named symbol * in the patched object. */ -static struct symbol *kpatch_find_static_twin(struct section *sec, +static struct symbol *kpatch_find_static_twin(struct section *relasec, struct symbol *sym) { struct symbol *res; - if (!sec->twin && sec->base->sym) { + if (!relasec->twin && relasec->base->sym) { struct symbol *parent = NULL; /* * The static twin might have been in a .part. symbol in the * original object that got removed in the patched object. */ - parent = kpatch_get_correlated_parent(sec->base->sym); + parent = kpatch_get_correlated_parent(relasec->base->sym); if (parent) - sec = parent->sec->rela; + relasec = parent->sec->rela; } - if (!sec->twin) + if (!relasec->twin) return NULL; - res = kpatch_find_uncorrelated_rela(sec->twin, sym); + res = kpatch_find_uncorrelated_rela(relasec->twin, sym); if (res != NULL) return res; /* Look if reference might have moved to child functions' sections */ - if (sec->twin->base->sym) - return kpatch_find_static_twin_in_children(sec->twin->base->sym, + if (relasec->twin->base->sym) + return kpatch_find_static_twin_in_children(relasec->twin->base->sym, sym); return NULL; @@ -1248,18 +1248,19 @@ static bool kpatch_is_normal_static_local(struct symbol *sym) return true; } -static struct rela *kpatch_find_static_twin_ref(struct section *rela_sec, struct symbol *sym) +static struct rela *kpatch_find_static_twin_ref(struct section *relasec, + struct symbol *sym) { struct rela *rela; - list_for_each_entry(rela, &rela_sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (rela->sym == sym->twin) return rela; } /* Reference to static variable might have moved to child function section */ - if (rela_sec->base->sym) { - struct symbol *parent = rela_sec->base->sym; + if (relasec->base->sym) { + struct symbol *parent = relasec->base->sym; struct symbol *child; list_for_each_entry(child, &parent->children, subfunction_node) { @@ -1305,7 +1306,7 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, struct kpatch_elf *patched) { struct symbol *sym, *patched_sym; - struct section *sec; + struct section *relasec; struct rela *rela; int bundled, patched_bundled; @@ -1338,14 +1339,14 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, * Do the correlations: for each section reference to a static local, * look for a corresponding reference in the section's twin. */ - list_for_each_entry(sec, &orig->sections, list) { + list_for_each_entry(relasec, &orig->sections, list) { - if (!is_rela_section(sec) || - is_debug_section(sec) || - !strcmp(sec->name, ".rela.toc")) + if (!is_rela_section(relasec) || + is_debug_section(relasec) || + !strcmp(relasec->name, ".rela.toc")) continue; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (!toc_rela(rela)) continue; /* skip toc constants */ @@ -1358,7 +1359,7 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, continue; bundled = sym == sym->sec->sym; - if (bundled && sym->sec == sec->base) { + if (bundled && sym->sec == relasec->base) { /* * A rare case where a static local data * structure references itself. There's no @@ -1371,11 +1372,11 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, continue; } - patched_sym = kpatch_find_static_twin(sec, sym); + patched_sym = kpatch_find_static_twin(relasec, sym); if (!patched_sym) DIFF_FATAL("reference to static local variable %s in %s was removed", sym->name, - kpatch_section_function_name(sec)); + kpatch_section_function_name(relasec)); patched_bundled = patched_sym == patched_sym->sec->sym; if (bundled != patched_bundled) @@ -1401,23 +1402,23 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, * corresponding reference in the patched object (because a static * local can be referenced by more than one section). */ - list_for_each_entry(sec, &orig->sections, list) { + list_for_each_entry(relasec, &orig->sections, list) { - if (!is_rela_section(sec) || - is_debug_section(sec)) + if (!is_rela_section(relasec) || + is_debug_section(relasec)) continue; - list_for_each_entry(rela, &sec->relas, list) { - struct section *target_sec = sec; + list_for_each_entry(rela, &relasec->relas, list) { + struct section *target_sec = relasec; sym = rela->sym; if (!kpatch_is_normal_static_local(sym)) continue; - if (!sec->twin && sec->base->sym) { + if (!relasec->twin && relasec->base->sym) { struct symbol *parent = NULL; - parent = kpatch_get_correlated_parent(sec->base->sym); + parent = kpatch_get_correlated_parent(relasec->base->sym); if (parent) target_sec = parent->sec->rela; } @@ -1439,13 +1440,13 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, * static locals to see if we need to print any warnings about new * variables. */ - list_for_each_entry(sec, &patched->sections, list) { + list_for_each_entry(relasec, &patched->sections, list) { - if (!is_rela_section(sec) || - is_debug_section(sec)) + if (!is_rela_section(relasec) || + is_debug_section(relasec)) continue; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { sym = rela->sym; if (!kpatch_is_normal_static_local(sym)) @@ -1456,7 +1457,7 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, log_normal("WARNING: unable to correlate static local variable %s used by %s, assuming variable is new\n", sym->name, - kpatch_section_function_name(sec)); + kpatch_section_function_name(relasec)); return; } } @@ -1476,13 +1477,13 @@ static void kpatch_compare_correlated_elements(struct kpatch_elf *kelf) kpatch_compare_symbols(&kelf->symbols); } -static void rela_insn(const struct section *sec, const struct rela *rela, +static void rela_insn(const struct section *relasec, const struct rela *rela, struct insn *insn) { unsigned long insn_addr, start, end, rela_addr; - start = (unsigned long)sec->base->data->d_buf; - end = start + sec->base->sh.sh_size; + start = (unsigned long)relasec->base->data->d_buf; + end = start + relasec->base->sh.sh_size; if (end <= start) ERROR("bad section size"); @@ -1493,7 +1494,7 @@ static void rela_insn(const struct section *sec, const struct rela *rela, insn_get_length(insn); if (!insn->length) ERROR("can't decode instruction in section %s at offset 0x%lx", - sec->base->name, insn_addr); + relasec->base->name, insn_addr); if (rela_addr >= insn_addr && rela_addr < insn_addr + insn->length) return; @@ -1531,19 +1532,19 @@ static bool is_callback_section(struct section *sec) { */ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) { - struct section *sec; + struct section *relasec; struct rela *rela; struct symbol *sym; unsigned int add_off; log_debug("\n"); - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec) || - is_debug_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec) || + is_debug_section(relasec)) continue; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (rela->sym->type != STT_SECTION || !rela->sym->sec) continue; @@ -1577,9 +1578,9 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) if (rela->type == R_X86_64_PC32 || rela->type == R_X86_64_PLT32) { struct insn insn; - rela_insn(sec, rela, &insn); + rela_insn(relasec, rela, &insn); add_off = (unsigned int)((long)insn.next_byte - - (long)sec->base->data->d_buf - + (long)relasec->base->data->d_buf - rela->offset); } else if (rela->type == R_X86_64_64 || rela->type == R_X86_64_32S) @@ -1647,7 +1648,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) continue; log_debug("%s: replacing %s+%ld reference with %s+%ld\n", - sec->name, + relasec->name, rela->sym->name, rela->addend, sym->name, rela->addend - start); @@ -2164,17 +2165,17 @@ static int fixup_barrier_nospec_group_size(struct kpatch_elf *kelf, int offset) */ static int fixup_group_size(struct kpatch_elf *kelf, int offset) { - struct section *sec; + struct section *relasec; struct rela *rela; int found; - sec = find_section_by_name(&kelf->sections, ".rela__ex_table"); - if (!sec) + relasec = find_section_by_name(&kelf->sections, ".rela__ex_table"); + if (!relasec) ERROR("missing .rela__ex_table section"); /* find beginning of this group */ found = 0; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (!strcmp(rela->sym->name, ".fixup") && rela->addend == offset) { found = 1; @@ -2187,7 +2188,7 @@ static int fixup_group_size(struct kpatch_elf *kelf, int offset) /* find beginning of next group */ found = 0; - list_for_each_entry_continue(rela, &sec->relas, list) { + list_for_each_entry_continue(rela, &relasec->relas, list) { if (!strcmp(rela->sym->name, ".fixup") && rela->addend > offset) { found = 1; @@ -2287,7 +2288,7 @@ static struct special_section special_sections[] = { }; static bool should_keep_jump_label(struct lookup_table *lookup, - struct section *sec, + struct section *relasec, unsigned int group_offset, unsigned int group_size, int *jump_labels_found) @@ -2302,7 +2303,7 @@ static bool should_keep_jump_label(struct lookup_table *lookup, * struct. It has three fields: code, target, and key. Each field has * a relocation associated with it. */ - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (rela->offset >= group_offset && rela->offset < group_offset + group_size) { if (i == 0) @@ -2389,29 +2390,29 @@ static bool should_keep_jump_label(struct lookup_table *lookup, } static bool should_keep_rela_group(struct lookup_table *lookup, - struct section *sec, unsigned int offset, + struct section *relasec, unsigned int offset, unsigned int size, int *jump_labels_found) { struct rela *rela; bool found = false; /* check if any relas in the group reference any changed functions */ - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (rela->offset >= offset && rela->offset < offset + size && rela->sym->type == STT_FUNC && rela->sym->sec->include) { found = true; log_debug("new/changed symbol %s found in special section %s\n", - rela->sym->name, sec->name); + rela->sym->name, relasec->name); } } if (!found) return false; - if (!strcmp(sec->name, ".rela__jump_table")) - return should_keep_jump_label(lookup, sec, offset, size, + if (!strcmp(relasec->name, ".rela__jump_table")) + return should_keep_jump_label(lookup, relasec, offset, size, jump_labels_found); return true; @@ -2428,13 +2429,13 @@ static void kpatch_update_ex_table_addend(struct kpatch_elf *kelf, int group_size) { struct rela *rela; - struct section *sec; + struct section *relasec; - sec = find_section_by_name(&kelf->sections, ".rela__ex_table"); - if (!sec) + relasec = find_section_by_name(&kelf->sections, ".rela__ex_table"); + if (!relasec) ERROR("missing .rela__ex_table section"); - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (!strcmp(rela->sym->name, ".fixup") && rela->addend >= src_offset && rela->addend < src_offset + group_size) @@ -2445,7 +2446,7 @@ static void kpatch_update_ex_table_addend(struct kpatch_elf *kelf, static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, struct lookup_table *lookup, struct special_section *special, - struct section *sec) + struct section *relasec) { struct rela *rela, *safe; char *src, *dest; @@ -2454,15 +2455,15 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, LIST_HEAD(newrelas); - src = sec->base->data->d_buf; + src = relasec->base->data->d_buf; /* alloc buffer for new base section */ - dest = malloc(sec->base->sh.sh_size); + dest = malloc(relasec->base->sh.sh_size); if (!dest) ERROR("malloc"); /* Restore the stashed r_addend from kpatch_update_ex_table_addend. */ if (!strcmp(special->name, "__ex_table")) { - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (!strcmp(rela->sym->name, ".fixup")) rela->addend = rela->rela.r_addend; } @@ -2470,7 +2471,7 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, src_offset = 0; dest_offset = 0; - for ( ; src_offset < sec->base->sh.sh_size; src_offset += group_size) { + for ( ; src_offset < relasec->base->sh.sh_size; src_offset += group_size) { group_size = special->group_size(kelf, src_offset); @@ -2482,10 +2483,10 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, * contains the data but doesn't go past the end of the * section. */ - if (src_offset + group_size > sec->base->sh.sh_size) - group_size = (unsigned int)(sec->base->sh.sh_size - src_offset); + if (src_offset + group_size > relasec->base->sh.sh_size) + group_size = (unsigned int)(relasec->base->sh.sh_size - src_offset); - if (!should_keep_rela_group(lookup, sec, src_offset, group_size, + if (!should_keep_rela_group(lookup, relasec, src_offset, group_size, &jump_labels_found)) continue; @@ -2494,7 +2495,7 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, * aren't sorted (e.g. .rela.fixup), so go through the entire * rela list each time. */ - list_for_each_entry_safe(rela, safe, &sec->relas, list) { + list_for_each_entry_safe(rela, safe, &relasec->relas, list) { if (rela->offset >= src_offset && rela->offset < src_offset + group_size) { /* copy rela entry */ @@ -2525,21 +2526,21 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, if (!dest_offset) { /* no changed or global functions referenced */ - sec->status = sec->base->status = SAME; - sec->include = sec->base->include = 0; + relasec->status = relasec->base->status = SAME; + relasec->include = relasec->base->include = 0; free(dest); return; } /* overwrite with new relas list */ - list_replace(&newrelas, &sec->relas); + list_replace(&newrelas, &relasec->relas); /* include both rela and base sections */ - sec->include = 1; - sec->base->include = 1; + relasec->include = 1; + relasec->base->include = 1; /* include secsym so .kpatch.arch relas can point to section symbols */ - if (sec->base->secsym) - sec->base->secsym->include = 1; + if (relasec->base->secsym) + relasec->base->secsym->include = 1; /* * Update text section data buf and size. @@ -2547,8 +2548,8 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf, * The rela section's data buf and size will be regenerated in * kpatch_rebuild_rela_section_data(). */ - sec->base->data->d_buf = dest; - sec->base->data->d_size = dest_offset; + relasec->base->data->d_buf = dest; + relasec->base->data->d_size = dest_offset; } #define ORC_IP_PTR_SIZE 4 @@ -2645,18 +2646,18 @@ next: static void kpatch_check_relocations(struct kpatch_elf *kelf) { struct rela *rela; - struct section *sec; + struct section *relasec; Elf_Data *sdata; - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) continue; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { if (rela->sym->sec) { sdata = rela->sym->sec->data; if ((long)rela->sym->sym.st_value + rela->addend > (long)sdata->d_size) { ERROR("out-of-range relocation %s+%lx in %s", rela->sym->name, - rela->addend, sec->name); + rela->addend, relasec->name); } } } @@ -3076,11 +3077,11 @@ static int function_ptr_rela(const struct rela *rela) } static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table, - struct section *sec, const struct rela *rela) + struct section *relasec, const struct rela *rela) { struct lookup_result symbol; - if (is_debug_section(sec)) + if (is_debug_section(relasec)) return false; /* @@ -3092,7 +3093,7 @@ static bool need_dynrela(struct kpatch_elf *kelf, struct lookup_table *table, return false; /* v5.13+ kernels use relative jump labels */ - if (rela->type == R_PPC64_REL64 && strcmp(sec->name, ".rela__jump_table")) + if (rela->type == R_PPC64_REL64 && strcmp(relasec->name, ".rela__jump_table")) return false; /* @@ -3249,7 +3250,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, char *pmod_name) { int nr, index; - struct section *sec, *ksym_sec, *krela_sec; + struct section *relasec, *ksym_sec, *krela_sec; struct rela *rela, *rela2, *safe; struct symbol *strsym, *ksym_sec_sym; struct kpatch_symbol *ksyms; @@ -3261,12 +3262,12 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, /* count rela entries that need to be dynamic */ nr = 0; - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) continue; - if (!strcmp(sec->name, ".rela.kpatch.funcs")) + if (!strcmp(relasec->name, ".rela.kpatch.funcs")) continue; - list_for_each_entry(rela, &sec->relas, list) { + list_for_each_entry(rela, &relasec->relas, list) { /* upper bound on number of kpatch relas and symbols */ nr++; @@ -3282,7 +3283,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, * internal symbol function pointer check which is done * via .toc indirection in need_dynrela(). */ - if (need_dynrela(kelf, table, sec, rela)) + if (need_dynrela(kelf, table, relasec, rela)) toc_rela(rela)->need_dynrela = 1; } } @@ -3310,12 +3311,12 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, /* populate sections */ index = 0; - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) continue; - if (!strcmp(sec->name, ".rela.kpatch.funcs") || - !strcmp(sec->name, ".rela.kpatch.relocations") || - !strcmp(sec->name, ".rela.kpatch.symbols")) + if (!strcmp(relasec->name, ".rela.kpatch.funcs") || + !strcmp(relasec->name, ".rela.kpatch.relocations") || + !strcmp(relasec->name, ".rela.kpatch.symbols")) continue; special = false; @@ -3323,11 +3324,11 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, if ((s->arch & kelf->arch) == 0) continue; - if (!strcmp(sec->base->name, s->name)) + if (!strcmp(relasec->base->name, s->name)) special = true; } - list_for_each_entry_safe(rela, safe, &sec->relas, list) { + list_for_each_entry_safe(rela, safe, &relasec->relas, list) { if (!rela->need_dynrela) { rela->sym->strip = SYMBOL_USED; continue; @@ -3348,7 +3349,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, */ if (!KLP_ARCH && !vmlinux && special) ERROR("unsupported dynrela reference to symbol '%s' in module-specific special section '%s'", - rela->sym->name, sec->base->name); + rela->sym->name, relasec->base->name); if (!lookup_symbol(table, rela->sym, &symbol)) ERROR("can't find symbol '%s' in symbol table", @@ -3394,11 +3395,11 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, /* add rela to fill in krelas[index].dest field */ ALLOC_LINK(rela2, &krela_sec->rela->relas); - if (sec->base->secsym) - rela2->sym = sec->base->secsym; + if (relasec->base->secsym) + rela2->sym = relasec->base->secsym; else ERROR("can't create dynrela for section %s (symbol %s): no bundled or section symbol", - sec->name, rela->sym->name); + relasec->name, rela->sym->name); rela2->type = absolute_rela_type(kelf); rela2->addend = rela->offset; @@ -3833,7 +3834,7 @@ int main(int argc, char *argv[]) struct arguments arguments; int num_changed, callbacks_exist, new_globals_exist; struct lookup_table *lookup; - struct section *sec, *symtab; + struct section *relasec, *symtab; char *orig_obj, *patched_obj, *parent_name; char *parent_symtab, *mod_symvers, *patch_name, *output_obj; @@ -3958,12 +3959,12 @@ int main(int argc, char *argv[]) if (!symtab) ERROR("missing .symtab section"); - list_for_each_entry(sec, &kelf_out->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf_out->sections, list) { + if (!is_rela_section(relasec)) continue; - sec->sh.sh_link = symtab->index; - sec->sh.sh_info = sec->base->index; - kpatch_rebuild_rela_section_data(sec); + relasec->sh.sh_link = symtab->index; + relasec->sh.sh_info = relasec->base->index; + kpatch_rebuild_rela_section_data(relasec); } kpatch_check_relocations(kelf_out); diff --git a/kpatch-build/create-klp-module.c b/kpatch-build/create-klp-module.c index c036b94..e942b9e 100644 --- a/kpatch-build/create-klp-module.c +++ b/kpatch-build/create-klp-module.c @@ -122,37 +122,37 @@ static struct section *find_or_add_klp_relasec(struct kpatch_elf *kelf, struct section *base, char *objname) { - struct section *sec; + struct section *relasec; char buf[256]; /* .klp.rela.objname.secname */ snprintf(buf, 256, KLP_RELASEC_PREFIX "%s.%s", objname, base->name); - list_for_each_entry(sec, &kelf->sections, list) { - if (!strcmp(sec->name, buf)) - return sec; + list_for_each_entry(relasec, &kelf->sections, list) { + if (!strcmp(relasec->name, buf)) + return relasec; } - ALLOC_LINK(sec, &kelf->sections); - sec->name = strdup(buf); - if (!sec->name) + ALLOC_LINK(relasec, &kelf->sections); + relasec->name = strdup(buf); + if (!relasec->name) ERROR("strdup"); - sec->base = base; + relasec->base = base; - INIT_LIST_HEAD(&sec->relas); + INIT_LIST_HEAD(&relasec->relas); - sec->data = malloc(sizeof(*sec->data)); - if (!sec->data) + relasec->data = malloc(sizeof(*relasec->data)); + if (!relasec->data) ERROR("malloc"); - sec->data->d_type = ELF_T_RELA; + relasec->data->d_type = ELF_T_RELA; /* sh_info and sh_link are set when rebuilding rela sections */ - sec->sh.sh_type = SHT_RELA; - sec->sh.sh_entsize = sizeof(GElf_Rela); - sec->sh.sh_addralign = 8; - sec->sh.sh_flags = SHF_RELA_LIVEPATCH | SHF_INFO_LINK | SHF_ALLOC; + relasec->sh.sh_type = SHT_RELA; + relasec->sh.sh_entsize = sizeof(GElf_Rela); + relasec->sh.sh_addralign = 8; + relasec->sh.sh_flags = SHF_RELA_LIVEPATCH | SHF_INFO_LINK | SHF_ALLOC; - return sec; + return relasec; } /* @@ -429,7 +429,7 @@ static struct argp argp = { options, parse_opt, args_doc, 0 }; int main(int argc, char *argv[]) { struct kpatch_elf *kelf; - struct section *symtab, *sec; + struct section *symtab, *relasec; struct section *ksymsec, *krelasec, *strsec; struct arguments arguments; char *strings; @@ -493,12 +493,12 @@ int main(int argc, char *argv[]) if (!symtab) ERROR("missing .symtab section"); - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) continue; - sec->sh.sh_link = symtab->index; - sec->sh.sh_info = sec->base->index; - kpatch_rebuild_rela_section_data(sec); + relasec->sh.sh_link = symtab->index; + relasec->sh.sh_info = relasec->base->index; + kpatch_rebuild_rela_section_data(relasec); } kpatch_create_shstrtab(kelf); diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index 1de8613..8a95259 100644 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -164,7 +164,8 @@ int offset_of_string(struct list_head *list, char *name) return index; } -static void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec) +static void kpatch_create_rela_list(struct kpatch_elf *kelf, + struct section *relasec) { int index = 0, skip = 0; struct rela *rela; @@ -172,28 +173,28 @@ static void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *sec unsigned long rela_nr; /* find matching base (text/data) section */ - sec->base = find_section_by_index(&kelf->sections, sec->sh.sh_info); - if (!sec->base) - ERROR("can't find base section for rela section %s", sec->name); + relasec->base = find_section_by_index(&kelf->sections, relasec->sh.sh_info); + if (!relasec->base) + ERROR("can't find base section for rela section %s", relasec->name); /* create reverse link from base section to this rela section */ - sec->base->rela = sec; + relasec->base->rela = relasec; - rela_nr = sec->sh.sh_size / sec->sh.sh_entsize; + rela_nr = relasec->sh.sh_size / relasec->sh.sh_entsize; log_debug("\n=== rela list for %s (%ld entries) ===\n", - sec->base->name, rela_nr); + relasec->base->name, rela_nr); - if (is_debug_section(sec)) { + if (is_debug_section(relasec)) { log_debug("skipping rela listing for .debug_* section\n"); skip = 1; } /* read and store the rela entries */ while (rela_nr--) { - ALLOC_LINK(rela, &sec->relas); + ALLOC_LINK(rela, &relasec->relas); - if (!gelf_getrela(sec->data, index, &rela->rela)) + if (!gelf_getrela(relasec->data, index, &rela->rela)) ERROR("gelf_getrela"); index++; @@ -346,7 +347,7 @@ struct kpatch_elf *kpatch_elf_open(const char *name) Elf *elf; int fd; struct kpatch_elf *kelf; - struct section *sec; + struct section *relasec; GElf_Ehdr ehdr; fd = open(name, O_RDONLY); @@ -372,11 +373,11 @@ struct kpatch_elf *kpatch_elf_open(const char *name) kpatch_create_symbol_list(kelf); /* for each rela section, read and store the rela entries */ - list_for_each_entry(sec, &kelf->sections, list) { - if (!is_rela_section(sec)) + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) continue; - INIT_LIST_HEAD(&sec->relas); - kpatch_create_rela_list(kelf, sec); + INIT_LIST_HEAD(&relasec->relas); + kpatch_create_rela_list(kelf, relasec); } if (!gelf_getehdr(kelf->elf, &ehdr)) From 61e46094b5da413087eb2ac8b4e09104f1c4d5ad Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:11:57 -0700 Subject: [PATCH 04/17] create-diff-object: convert function return types to 'bool' Several functions have a boolean semantic, but don't actually return bool, which is confusing. Fix that. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 32 +++++++++++++++---------------- kpatch-build/kpatch-elf.c | 14 +++++++------- kpatch-build/kpatch-elf.h | 14 +++++++------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 903b92c..afe1bf6 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -85,51 +85,51 @@ struct special_section { * Functions * **********/ -static int is_bundleable(struct symbol *sym) +static bool is_bundleable(struct symbol *sym) { if (sym->type == STT_FUNC && !strncmp(sym->sec->name, ".text.",6) && !strcmp(sym->sec->name + 6, sym->name)) - return 1; + return true; if (sym->type == STT_FUNC && !strncmp(sym->sec->name, ".text.unlikely.",15) && (!strcmp(sym->sec->name + 15, sym->name) || (strstr(sym->name, ".cold") && !strncmp(sym->sec->name + 15, sym->name, strlen(sym->sec->name) - 15)))) - return 1; + return true; if (sym->type == STT_FUNC && !strncmp(sym->sec->name, ".text.hot.",10) && !strcmp(sym->sec->name + 10, sym->name)) - return 1; + return true; if (sym->type == STT_OBJECT && !strncmp(sym->sec->name, ".data.",6) && !strcmp(sym->sec->name + 6, sym->name)) - return 1; + return true; if (sym->type == STT_OBJECT && !strncmp(sym->sec->name, ".data.rel.", 10) && !strcmp(sym->sec->name + 10, sym->name)) - return 1; + return true; if (sym->type == STT_OBJECT && !strncmp(sym->sec->name, ".data.rel.ro.", 13) && !strcmp(sym->sec->name + 13, sym->name)) - return 1; + return true; if (sym->type == STT_OBJECT && !strncmp(sym->sec->name, ".rodata.",8) && !strcmp(sym->sec->name + 8, sym->name)) - return 1; + return true; if (sym->type == STT_OBJECT && !strncmp(sym->sec->name, ".bss.",5) && !strcmp(sym->sec->name + 5, sym->name)) - return 1; + return true; - return 0; + return false; } /* Symbol st_others value for powerpc */ @@ -874,7 +874,7 @@ static enum subsection kpatch_subsection_type(struct section *sec) return SUBSECTION_NORMAL; } -static int kpatch_subsection_changed(struct section *sec1, struct section *sec2) +static bool kpatch_subsection_changed(struct section *sec1, struct section *sec2) { return kpatch_subsection_type(sec1) != kpatch_subsection_type(sec2); } @@ -1825,12 +1825,12 @@ static void kpatch_include_standard_elements(struct kpatch_elf *kelf) list_entry(kelf->symbols.next, struct symbol, list)->include = 1; } -static int kpatch_include_callback_elements(struct kpatch_elf *kelf) +static bool kpatch_include_callback_elements(struct kpatch_elf *kelf) { struct section *sec; struct symbol *sym; struct rela *rela; - int found = 0; + bool found = false; /* include load/unload sections */ list_for_each_entry(sec, &kelf->sections, list) { @@ -1838,7 +1838,7 @@ static int kpatch_include_callback_elements(struct kpatch_elf *kelf) continue; sec->include = 1; - found = 1; + found = true; if (is_rela_section(sec)) { /* include callback dependencies */ rela = list_entry(sec->relas.next, struct rela, list); @@ -1942,7 +1942,7 @@ static void kpatch_print_changes(struct kpatch_elf *kelf) static void kpatch_migrate_symbols(struct list_head *src, struct list_head *dst, - int (*select)(struct symbol *)) + bool (*select)(struct symbol *)) { struct symbol *sym, *safe; @@ -3057,7 +3057,7 @@ static void kpatch_create_patches_sections(struct kpatch_elf *kelf, } -static int kpatch_is_core_module_symbol(char *name) +static bool kpatch_is_core_module_symbol(char *name) { return (!strcmp(name, "kpatch_shadow_alloc") || !strcmp(name, "kpatch_shadow_free") || diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index 8a95259..793cbd8 100644 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -53,18 +53,18 @@ char *status_str(enum status status) return NULL; } -int is_rela_section(struct section *sec) +bool is_rela_section(struct section *sec) { return (sec->sh.sh_type == SHT_RELA); } -int is_text_section(struct section *sec) +bool is_text_section(struct section *sec) { return (sec->sh.sh_type == SHT_PROGBITS && (sec->sh.sh_flags & SHF_EXECINSTR)); } -int is_debug_section(struct section *sec) +bool is_debug_section(struct section *sec) { char *name; if (is_rela_section(sec)) @@ -443,22 +443,22 @@ next: } } -int is_null_sym(struct symbol *sym) +bool is_null_sym(struct symbol *sym) { return !strlen(sym->name); } -int is_file_sym(struct symbol *sym) +bool is_file_sym(struct symbol *sym) { return sym->type == STT_FILE; } -int is_local_func_sym(struct symbol *sym) +bool is_local_func_sym(struct symbol *sym) { return sym->bind == STB_LOCAL && sym->type == STT_FUNC; } -int is_local_sym(struct symbol *sym) +bool is_local_sym(struct symbol *sym) { return sym->bind == STB_LOCAL; } diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h index fbaa703..312de71 100644 --- a/kpatch-build/kpatch-elf.h +++ b/kpatch-build/kpatch-elf.h @@ -129,9 +129,9 @@ struct kpatch_elf { * Helper functions ******************/ char *status_str(enum status status); -int is_rela_section(struct section *sec); -int is_text_section(struct section *sec); -int is_debug_section(struct section *sec); +bool is_rela_section(struct section *sec); +bool is_text_section(struct section *sec); +bool is_debug_section(struct section *sec); struct section *find_section_by_index(struct list_head *list, unsigned int index); struct section *find_section_by_name(struct list_head *list, const char *name); @@ -163,10 +163,10 @@ int offset_of_string(struct list_head *list, char *name); struct kpatch_elf *kpatch_elf_open(const char *name); void kpatch_dump_kelf(struct kpatch_elf *kelf); -int is_null_sym(struct symbol *sym); -int is_file_sym(struct symbol *sym); -int is_local_func_sym(struct symbol *sym); -int is_local_sym(struct symbol *sym); +bool is_null_sym(struct symbol *sym); +bool is_file_sym(struct symbol *sym); +bool is_local_func_sym(struct symbol *sym); +bool is_local_sym(struct symbol *sym); void print_strtab(char *buf, size_t size); void kpatch_create_shstrtab(struct kpatch_elf *kelf); From 79f45d1b0afd2ff0c3235be7cc32f934453e7acb Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:34:44 -0700 Subject: [PATCH 05/17] create-diff-object: fix kpatch_replace_sections_syms() for non-text It doesn't make sense to disassemble a data section. That just happened to work by accident. PC-relative offsets only need adjusting when associated with an instruction. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index afe1bf6..03c3321 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1575,17 +1575,18 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) add_off = 0; break; case X86_64: - if (rela->type == R_X86_64_PC32 || - rela->type == R_X86_64_PLT32) { + if (!is_text_section(relasec->base) || + rela->type == R_X86_64_64 || + rela->type == R_X86_64_32S) + add_off = 0; + else if (rela->type == R_X86_64_PC32 || + rela->type == R_X86_64_PLT32) { struct insn insn; rela_insn(relasec, rela, &insn); add_off = (unsigned int)((long)insn.next_byte - (long)relasec->base->data->d_buf - rela->offset); - } else if (rela->type == R_X86_64_64 || - rela->type == R_X86_64_32S) - add_off = 0; - else + } else continue; break; default: From 3f8e1062cc0e0f782ba6793735d59b1e07116265 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:36:41 -0700 Subject: [PATCH 06/17] create-diff-object: support R_X86_64_NONE in kpatch_replace_sections_syms() Add support for R_X86_64_NONE. With an upstream kernel, it's quite rare, only used for a few jump labels. With older kernels it was used for fentry hooks. Either way, it should be treated like a PC-relative relocation. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 03c3321..4f1bb21 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1580,7 +1580,8 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) rela->type == R_X86_64_32S) add_off = 0; else if (rela->type == R_X86_64_PC32 || - rela->type == R_X86_64_PLT32) { + rela->type == R_X86_64_PLT32 || + rela->type == R_X86_64_NONE) { struct insn insn; rela_insn(relasec, rela, &insn); add_off = (unsigned int)((long)insn.next_byte - From bf212f77500eae994752a1c8feb38b007a4336f2 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:38:18 -0700 Subject: [PATCH 07/17] create-diff-object: error on unsupported rela in symbol conversion Error out if an unsupported rela is encountered. This is more robust than just ignoring it. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 4f1bb21..e4ea45b 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1588,7 +1588,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) (long)relasec->base->data->d_buf - rela->offset); } else - continue; + ERROR("unhandled rela type %d", rela->type); break; default: ERROR("unsupported arch"); From 6b1895a6b7ff3b826a7f4d2b6930d3607a65752b Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 14:50:43 -0700 Subject: [PATCH 08/17] create-diff-object: convert rela_insn() to take normal 'sec' rela_insn() only cares about the base section. Convert it to take a non-rela section as its argument instead of a relasec. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index e4ea45b..d8c4005 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1477,13 +1477,13 @@ static void kpatch_compare_correlated_elements(struct kpatch_elf *kelf) kpatch_compare_symbols(&kelf->symbols); } -static void rela_insn(const struct section *relasec, const struct rela *rela, +static void rela_insn(const struct section *sec, const struct rela *rela, struct insn *insn) { unsigned long insn_addr, start, end, rela_addr; - start = (unsigned long)relasec->base->data->d_buf; - end = start + relasec->base->sh.sh_size; + start = (unsigned long)sec->data->d_buf; + end = start + sec->sh.sh_size; if (end <= start) ERROR("bad section size"); @@ -1494,7 +1494,7 @@ static void rela_insn(const struct section *relasec, const struct rela *rela, insn_get_length(insn); if (!insn->length) ERROR("can't decode instruction in section %s at offset 0x%lx", - relasec->base->name, insn_addr); + sec->name, insn_addr); if (rela_addr >= insn_addr && rela_addr < insn_addr + insn->length) return; @@ -1583,7 +1583,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) rela->type == R_X86_64_PLT32 || rela->type == R_X86_64_NONE) { struct insn insn; - rela_insn(relasec, rela, &insn); + rela_insn(relasec->base, rela, &insn); add_off = (unsigned int)((long)insn.next_byte - (long)relasec->base->data->d_buf - rela->offset); From bec6488af6432fe7d514a34d87cd89ca4210cf01 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 15:00:24 -0700 Subject: [PATCH 09/17] create-diff-object: add rela_insn() error check Error out if the insn can't be found. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index d8c4005..1c2f584 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1499,6 +1499,9 @@ static void rela_insn(const struct section *sec, const struct rela *rela, rela_addr < insn_addr + insn->length) return; } + + ERROR("can't find instruction for rela at %s+0x%x", + sec->name, rela->offset); } static bool is_callback_section(struct section *sec) { From 01427d50a18ec0cca687fca0d02e6a41c0e95901 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:25:13 -0700 Subject: [PATCH 10/17] create-diff-object: move addend math to a new function Split out the addend offset math into a separate function so it can be used elsewhere. Signed-off-by: Josh Poimboeuf --- kpatch-build/Makefile | 4 +- kpatch-build/create-diff-object.c | 93 ++--------------------------- kpatch-build/kpatch-elf.c | 98 +++++++++++++++++++++++++++++++ kpatch-build/kpatch-elf.h | 3 + 4 files changed, 107 insertions(+), 91 deletions(-) diff --git a/kpatch-build/Makefile b/kpatch-build/Makefile index a874dd3..da96edf 100644 --- a/kpatch-build/Makefile +++ b/kpatch-build/Makefile @@ -31,8 +31,8 @@ all: $(TARGETS) -include $(SOURCES:.c=.d) create-diff-object: create-diff-object.o kpatch-elf.o lookup.o $(INSN) -create-klp-module: create-klp-module.o kpatch-elf.o -create-kpatch-module: create-kpatch-module.o kpatch-elf.o +create-klp-module: create-klp-module.o kpatch-elf.o $(INSN) +create-kpatch-module: create-kpatch-module.o kpatch-elf.o $(INSN) $(PLUGIN): gcc-plugins/ppc64le-plugin.c g++ $(PLUGIN_CFLAGS) $< -o $@ diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 1c2f584..3ad446c 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -48,7 +48,6 @@ #include "list.h" #include "lookup.h" -#include "asm/insn.h" #include "kpatch-patch.h" #include "kpatch-elf.h" #include "kpatch-intermediate.h" @@ -577,39 +576,6 @@ out: log_debug("section %s has changed\n", sec->name); } -static unsigned int insn_length(struct kpatch_elf *kelf, void *addr) -{ - struct insn decoded_insn; - char *insn = addr; - - switch(kelf->arch) { - - case X86_64: - insn_init(&decoded_insn, addr, 1); - insn_get_length(&decoded_insn); - return decoded_insn.length; - - case PPC64: - return 4; - - case S390: - switch(insn[0] >> 6) { - case 0: - return 2; - case 1: - case 2: - return 4; - case 3: - return 6; - } - - default: - ERROR("unsupported arch"); - } - - return 0; -} - /* * This function is not comprehensive, i.e. it doesn't detect immediate loads * to *all* registers. It only detects those which have been found in the wild @@ -1477,33 +1443,6 @@ static void kpatch_compare_correlated_elements(struct kpatch_elf *kelf) kpatch_compare_symbols(&kelf->symbols); } -static void rela_insn(const struct section *sec, const struct rela *rela, - struct insn *insn) -{ - unsigned long insn_addr, start, end, rela_addr; - - start = (unsigned long)sec->data->d_buf; - end = start + sec->sh.sh_size; - - if (end <= start) - ERROR("bad section size"); - - rela_addr = start + rela->offset; - for (insn_addr = start; insn_addr < end; insn_addr += insn->length) { - insn_init(insn, (void *)insn_addr, 1); - insn_get_length(insn); - if (!insn->length) - ERROR("can't decode instruction in section %s at offset 0x%lx", - sec->name, insn_addr); - if (rela_addr >= insn_addr && - rela_addr < insn_addr + insn->length) - return; - } - - ERROR("can't find instruction for rela at %s+0x%x", - sec->name, rela->offset); -} - static bool is_callback_section(struct section *sec) { static char *callback_sections[] = { @@ -1538,13 +1477,12 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) struct section *relasec; struct rela *rela; struct symbol *sym; - unsigned int add_off; + long target_off; log_debug("\n"); list_for_each_entry(relasec, &kelf->sections, list) { - if (!is_rela_section(relasec) || - is_debug_section(relasec)) + if (!is_rela_section(relasec) || is_debug_section(relasec)) continue; list_for_each_entry(rela, &relasec->relas, list) { @@ -1573,29 +1511,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) continue; } - switch(kelf->arch) { - case PPC64: - add_off = 0; - break; - case X86_64: - if (!is_text_section(relasec->base) || - rela->type == R_X86_64_64 || - rela->type == R_X86_64_32S) - add_off = 0; - else if (rela->type == R_X86_64_PC32 || - rela->type == R_X86_64_PLT32 || - rela->type == R_X86_64_NONE) { - struct insn insn; - rela_insn(relasec->base, rela, &insn); - add_off = (unsigned int)((long)insn.next_byte - - (long)relasec->base->data->d_buf - - rela->offset); - } else - ERROR("unhandled rela type %d", rela->type); - break; - default: - ERROR("unsupported arch"); - } + target_off = rela_target_offset(kelf, relasec, rela); /* * Attempt to replace references to unbundled sections @@ -1648,8 +1564,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) * be the same as &var2. */ - } else if (rela->addend + add_off < start || - rela->addend + add_off >= end) + } else if (target_off < start || target_off >= end) continue; log_debug("%s: replacing %s+%ld reference with %s+%ld\n", diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index 793cbd8..dba53b5 100644 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -31,6 +31,7 @@ #include #include +#include "asm/insn.h" #include "kpatch-elf.h" /******************* @@ -164,6 +165,103 @@ int offset_of_string(struct list_head *list, char *name) return index; } +static void rela_insn(const struct section *sec, const struct rela *rela, + struct insn *insn) +{ + unsigned long insn_addr, start, end, rela_addr; + + start = (unsigned long)sec->data->d_buf; + end = start + sec->sh.sh_size; + + if (end <= start) + ERROR("bad section size"); + + rela_addr = start + rela->offset; + for (insn_addr = start; insn_addr < end; insn_addr += insn->length) { + insn_init(insn, (void *)insn_addr, 1); + insn_get_length(insn); + if (!insn->length) + ERROR("can't decode instruction in section %s at offset 0x%lx", + sec->name, insn_addr); + if (rela_addr >= insn_addr && + rela_addr < insn_addr + insn->length) + return; + } + + ERROR("can't find instruction for rela at %s+0x%x", + sec->name, rela->offset); +} + +/* + * Return the addend, adjusted for any PC-relative relocation trickery, to + * point to the relevant symbol offset. + */ +long rela_target_offset(struct kpatch_elf *kelf, struct section *relasec, + struct rela *rela) +{ + long add_off; + struct section *sec = relasec->base; + + switch(kelf->arch) { + case PPC64: + add_off = 0; + break; + case X86_64: + if (!is_text_section(sec) || + rela->type == R_X86_64_64 || + rela->type == R_X86_64_32S) + add_off = 0; + else if (rela->type == R_X86_64_PC32 || + rela->type == R_X86_64_PLT32 || + rela->type == R_X86_64_NONE) { + struct insn insn; + rela_insn(sec, rela, &insn); + add_off = (long)insn.next_byte - + (long)sec->data->d_buf - + rela->offset; + } else + ERROR("unhandled rela type %d", rela->type); + break; + default: + ERROR("unsupported arch\n"); + } + + return rela->addend + add_off; +} + +unsigned int insn_length(struct kpatch_elf *kelf, void *addr) +{ + struct insn decoded_insn; + char *insn = addr; + + switch(kelf->arch) { + + case X86_64: + insn_init(&decoded_insn, addr, 1); + insn_get_length(&decoded_insn); + return decoded_insn.length; + + case PPC64: + return 4; + + case S390: + switch(insn[0] >> 6) { + case 0: + return 2; + case 1: + case 2: + return 4; + case 3: + return 6; + } + + default: + ERROR("unsupported arch"); + } + + return 0; +} + static void kpatch_create_rela_list(struct kpatch_elf *kelf, struct section *relasec) { diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h index 312de71..3bc6e76 100644 --- a/kpatch-build/kpatch-elf.h +++ b/kpatch-build/kpatch-elf.h @@ -152,6 +152,9 @@ struct rela *find_rela_by_offset(struct section *relasec, unsigned int offset); unsigned int absolute_rela_type(struct kpatch_elf *kelf); int offset_of_string(struct list_head *list, char *name); +long rela_target_offset(struct kpatch_elf *kelf, struct section *relasec, + struct rela *rela); +unsigned int insn_length(struct kpatch_elf *kelf, void *addr); #ifndef R_PPC64_ENTRY #define R_PPC64_ENTRY 118 From 8d5a628bde7298991a64b27f4ee5bd25b663e52f Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:28:41 -0700 Subject: [PATCH 11/17] create-diff-object: add extra check for symbol conversion edge case This issue was only seen in in a text section. Explicitly check for that. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 3ad446c..c51f010 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1527,7 +1527,8 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) start = sym->sym.st_value; end = sym->sym.st_value + sym->sym.st_size; - if (!is_text_section(sym->sec) && + if (is_text_section(relasec->base) && + !is_text_section(sym->sec) && rela->type == R_X86_64_32S && rela->addend == (long)sym->sec->sh.sh_size && end == (long)sym->sec->sh.sh_size) { From 8508abd3b1824783e4d51329614f6eaf9a468041 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:33:07 -0700 Subject: [PATCH 12/17] create-diff-object: allow converstion of empty symbols Empty (zero-length) symbols are possible, allow kpatch_replace_sections_syms() to work on them. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index c51f010..cc57db9 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1564,6 +1564,12 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) * &(var1+sizeof(var1)) will always * be the same as &var2. */ + } else if (target_off == start && target_off == end) { + + /* + * Allow replacement for references to + * empty symbols. + */ } else if (target_off < start || target_off >= end) continue; From 325bccd89dd8bb1683106fdbcd9d1ebe53061ef6 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:33:55 -0700 Subject: [PATCH 13/17] create-diff-object: skip conversion for sections which never have symbols These sections don't have symbols. Don't even try to replace references to them with symbols. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index cc57db9..2d96598 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1490,6 +1490,17 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) if (rela->sym->type != STT_SECTION || !rela->sym->sec) continue; + /* + * These sections don't have symbols associated with + * them: + */ + if (!strcmp(rela->sym->name, ".toc") || + !strcmp(rela->sym->name, ".fixup") || + !strcmp(rela->sym->name, ".altinstr_replacement") || + !strcmp(rela->sym->name, ".altinstr_aux") || + !strcmp(rela->sym->name, ".text..refcount")) + continue; + /* * Replace references to bundled sections with their * symbols. From 86d5208b46819459da8a6e3f8411d8ade774a4f9 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:35:00 -0700 Subject: [PATCH 14/17] create-diff-object: error on symbol conversion failure If a section reference can't be converted to a symbol reference, error out to try to prevent unexpected behavior later on. There are a few sections for which a symbol is optional: .rodata and string literal sections. Don't warn about those. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 2d96598..fd4f67e 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -302,6 +302,12 @@ static bool is_dynamic_debug_symbol(struct symbol *sym) return false; } +static bool is_string_literal_section(struct section *sec) +{ + return !strncmp(sec->name, ".rodata.", 8) && + strstr(sec->name, ".str1."); +} + /* * This function detects whether the given symbol is a "special" static local * variable (for lack of a better term). @@ -1478,6 +1484,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) struct rela *rela; struct symbol *sym; long target_off; + bool found = false; log_debug("\n"); @@ -1589,11 +1596,18 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf) relasec->name, rela->sym->name, rela->addend, sym->name, rela->addend - start); + found = true; rela->sym = sym; rela->addend -= start; break; } + + if (!found && !is_string_literal_section(rela->sym->sec) && + strncmp(rela->sym->name, ".rodata", 7)) { + ERROR("%s+0x%x: can't find replacement symbol for %s+%ld reference", + relasec->base->name, rela->offset, rela->sym->name, rela->addend); + } } } log_debug("\n"); @@ -1711,12 +1725,6 @@ static void kpatch_include_symbol(struct symbol *sym) kpatch_include_section(sym->sec); } -static bool is_string_literal_section(struct section *sec) -{ - return !strncmp(sec->name, ".rodata.", 8) && - strstr(sec->name, ".str1."); -} - static void kpatch_include_standard_elements(struct kpatch_elf *kelf) { struct section *sec; From f0e3da336c947d191a6b320bd1556b6a3a72dc1b Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 11 May 2022 16:53:28 -0700 Subject: [PATCH 15/17] create-diff-object: fix string extraction The current string extraction is broken for non-section symbols. Fix that. Signed-off-by: Josh Poimboeuf --- kpatch-build/kpatch-elf.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index dba53b5..6a43ef5 100644 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -305,7 +305,9 @@ static void kpatch_create_rela_list(struct kpatch_elf *kelf, ERROR("could not find rela entry symbol\n"); if (rela->sym->sec && (rela->sym->sec->sh.sh_flags & SHF_STRINGS)) { - rela->string = rela->sym->sec->data->d_buf + rela->addend; + rela->string = rela->sym->sec->data->d_buf + + rela->sym->sym.st_value + + rela_target_offset(kelf, relasec, rela); if (!rela->string) ERROR("could not lookup rela string for %s+%ld", rela->sym->name, rela->addend); @@ -467,16 +469,6 @@ struct kpatch_elf *kpatch_elf_open(const char *name) /* read and store section, symbol entries from file */ kelf->elf = elf; kelf->fd = fd; - kpatch_create_section_list(kelf); - kpatch_create_symbol_list(kelf); - - /* for each rela section, read and store the rela entries */ - list_for_each_entry(relasec, &kelf->sections, list) { - if (!is_rela_section(relasec)) - continue; - INIT_LIST_HEAD(&relasec->relas); - kpatch_create_rela_list(kelf, relasec); - } if (!gelf_getehdr(kelf->elf, &ehdr)) ERROR("gelf_getehdr"); @@ -490,6 +482,18 @@ struct kpatch_elf *kpatch_elf_open(const char *name) default: ERROR("Unsupported target architecture"); } + + kpatch_create_section_list(kelf); + kpatch_create_symbol_list(kelf); + + /* for each rela section, read and store the rela entries */ + list_for_each_entry(relasec, &kelf->sections, list) { + if (!is_rela_section(relasec)) + continue; + INIT_LIST_HEAD(&relasec->relas); + kpatch_create_rela_list(kelf, relasec); + } + return kelf; } From 017015a725f802b3fa8db6106cfe280e2b2cc91d Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Thu, 12 May 2022 06:02:15 -0700 Subject: [PATCH 16/17] create-diff-object: make kpatch_check_relocations() more precise Use rela_target_offset() to make the relocation bounds checking more precise. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index fd4f67e..0f06d84 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -2593,18 +2593,32 @@ static void kpatch_check_relocations(struct kpatch_elf *kelf) { struct rela *rela; struct section *relasec; - Elf_Data *sdata; + long sec_size; + long sec_off; list_for_each_entry(relasec, &kelf->sections, list) { if (!is_rela_section(relasec)) continue; list_for_each_entry(rela, &relasec->relas, list) { - if (rela->sym->sec) { - sdata = rela->sym->sec->data; - if ((long)rela->sym->sym.st_value + rela->addend > (long)sdata->d_size) { - ERROR("out-of-range relocation %s+%lx in %s", rela->sym->name, - rela->addend, relasec->name); - } + if (!rela->sym->sec) + continue; + + sec_size = rela->sym->sec->data->d_size; + sec_off = (long)rela->sym->sym.st_value + + rela_target_offset(kelf, relasec, rela); + + /* + * This check isn't perfect: we still allow relocations + * to the end of a section. There are real instances + * of that, including ORC entries, LOCKDEP=n + * zero-length '__key' passing, and the loop edge case + * described in kpatch_replace_sections_syms(). For + * now, just allow all such cases. + */ + if (sec_off < 0 || sec_off > sec_size) { + ERROR("%s+0x%x: out-of-range relocation %s+%lx", + relasec->base->name, rela->offset, + rela->sym->name, rela->addend); } } } From 52863dace0200332dc2ed5d8edab1f017e63b368 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Thu, 12 May 2022 06:25:32 -0700 Subject: [PATCH 17/17] create-diff-object: fix endianness in kpatch_no_sibling_calls_ppc64le() Otherwise it fails the unit tests on an s390 host. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 0f06d84..bebe3bd 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -3667,7 +3667,7 @@ static void kpatch_build_strings_section_data(struct kpatch_elf *kelf) static void kpatch_no_sibling_calls_ppc64le(struct kpatch_elf *kelf) { struct symbol *sym; - unsigned int insn; + unsigned char *insn; unsigned int offset; if (kelf->arch != PPC64) @@ -3679,7 +3679,7 @@ static void kpatch_no_sibling_calls_ppc64le(struct kpatch_elf *kelf) for (offset = 0; offset < sym->sec->data->d_size; offset += 4) { - insn = *(unsigned int *)(sym->sec->data->d_buf + offset); + insn = sym->sec->data->d_buf + offset; /* * The instruction 0x48000000 can be assumed to be a @@ -3694,7 +3694,8 @@ static void kpatch_no_sibling_calls_ppc64le(struct kpatch_elf *kelf) * there's a REL24 relocation for the address which * will be written by the linker or the kernel. */ - if (insn != 0x48000000) + if (insn[3] != 0x48 || insn[2] != 0x00 || + insn[1] != 0x00 || insn[0] != 0x00) continue; /* Make sure it's not a branch-to-self: */