From 9ea87f89d557754dba24445f08763eeeb18c693b Mon Sep 17 00:00:00 2001 From: Puranjay Mohan Date: Wed, 9 Apr 2025 12:33:24 +0000 Subject: [PATCH] create-diff-object: Fix ordering of __patchable_function_entries Old linkers don't support mixing ordered and unordered sections. This was fixed in binutils, see discussion in [1]. But without this fix, kpatch-build fails with an error like: | ld.bfd: __patchable_function_entries has both ordered | [`__patchable_function_entries' in /builddir/.kpatch/tmp/patch/output.o] | and unordered [`__patchable_function_entries' in | /builddir/.kpatch/tmp/patch/patch-hook.o] sections | ld.bfd: final link failed: bad value Fix this by only setting SHF_LINK_ORDER in the output object if the patched object also has this flag set. Signed-off-by: Puranjay Mohan --- kpatch-build/create-diff-object.c | 26 +++++++++++++------------- kpatch-build/kpatch-elf.c | 13 +++++++++++-- kpatch-build/kpatch-elf.h | 1 + 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index a5473b0a..c784bf1f 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1992,6 +1992,8 @@ static void kpatch_migrate_included_elements(struct kpatch_elf *kelf, struct kpa ERROR("malloc"); memset(out, 0, sizeof(*out)); out->arch = kelf->arch; + out->has_pfe = kelf->has_pfe; + out->pfe_ordered = kelf->pfe_ordered; INIT_LIST_HEAD(&out->sections); INIT_LIST_HEAD(&out->symbols); INIT_LIST_HEAD(&out->strings); @@ -3729,7 +3731,7 @@ static void kpatch_set_pfe_link(struct kpatch_elf *kelf) * TODO: Eventually we can modify recordmount so that it recognizes our bundled * sections as valid and does this work for us. */ -static void kpatch_create_ftrace_callsite_sections(struct kpatch_elf *kelf, bool has_pfe) +static void kpatch_create_ftrace_callsite_sections(struct kpatch_elf *kelf) { int nr, index; struct section *sec = NULL; @@ -3745,13 +3747,7 @@ static void kpatch_create_ftrace_callsite_sections(struct kpatch_elf *kelf, bool sym->has_func_profiling) nr++; - if (has_pfe) - /* - * Create separate __patchable_function_entries sections - * for each function in the following loop. - */ - kelf->has_pfe = true; - else + if (!kelf->has_pfe) /* * Create a single __mcount_loc section pair for all * functions. @@ -3858,7 +3854,14 @@ static void kpatch_create_ftrace_callsite_sections(struct kpatch_elf *kelf, bool * - its lone rela is based on the section symbol */ sec = create_section_pair(kelf, "__patchable_function_entries", sizeof(void *), 1); - sec->sh.sh_flags |= SHF_WRITE | SHF_ALLOC | SHF_LINK_ORDER; + sec->sh.sh_flags |= SHF_WRITE | SHF_ALLOC; + /* + * Old linkers don't support mixing ordered and unordered sections, so set + * the SHF_LINK_ORDER flag only if it is set in the pfe section generated by + * the compiler. + */ + if (kelf->pfe_ordered) + sec->sh.sh_flags |= SHF_LINK_ORDER; rela_sym = sym->sec->secsym; rela_offset = 0; rela_sym->pfe = sec; @@ -4158,7 +4161,6 @@ int main(int argc, char *argv[]) struct section *relasec, *symtab; char *orig_obj, *patched_obj, *parent_name; char *parent_symtab, *mod_symvers, *patch_name, *output_obj; - bool has_pfe = false; memset(&arguments, 0, sizeof(arguments)); argp_parse (&argp, argc, argv, 0, NULL, &arguments); @@ -4184,8 +4186,6 @@ int main(int argc, char *argv[]) kpatch_set_pfe_link(kelf_orig); kpatch_set_pfe_link(kelf_patched); - if (kelf_patched->has_pfe) - has_pfe = true; kpatch_find_func_profiling_calls(kelf_orig); kpatch_find_func_profiling_calls(kelf_patched); @@ -4266,7 +4266,7 @@ int main(int argc, char *argv[]) kpatch_create_callbacks_objname_rela(kelf_out, parent_name); kpatch_build_strings_section_data(kelf_out); - kpatch_create_ftrace_callsite_sections(kelf_out, has_pfe); + kpatch_create_ftrace_callsite_sections(kelf_out); /* * At this point, the set of output sections and symbols is diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c index c8ab5bd3..8c49bcb3 100755 --- a/kpatch-build/kpatch-elf.c +++ b/kpatch-build/kpatch-elf.c @@ -559,6 +559,7 @@ struct kpatch_elf *kpatch_elf_open(const char *name) int fd; struct kpatch_elf *kelf; struct section *relasec; + struct section *pfesec; GElf_Ehdr ehdr; fd = open(name, O_RDONLY); @@ -614,9 +615,17 @@ struct kpatch_elf *kpatch_elf_open(const char *name) * These sections aren't used by ftrace on this arch, so do not * bother reading/writing them for x86_64. */ - if (kelf->arch != X86_64) - if (find_section_by_name(&kelf->sections, "__patchable_function_entries")) + if (kelf->arch != X86_64) { + pfesec = find_section_by_name(&kelf->sections, "__patchable_function_entries"); + if (pfesec) { kelf->has_pfe = true; + /* + * Check if the pfe section generated by the toolchain has the SHF_LINK_ORDER + * flag set. This info is used while generating pfe sections in the diff object. + */ + kelf->pfe_ordered = !!(pfesec->sh.sh_flags & SHF_LINK_ORDER); + } + } return kelf; } diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h index e47acf97..6f5a52ef 100644 --- a/kpatch-build/kpatch-elf.h +++ b/kpatch-build/kpatch-elf.h @@ -127,6 +127,7 @@ struct kpatch_elf { Elf_Data *symtab_shndx; int fd; bool has_pfe; + bool pfe_ordered; }; /*******************