From 72e260f50c74c4e61da2f26964245b1ee470ebbe Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 3 Jun 2014 09:30:33 -0500 Subject: [PATCH 1/2] create-diff-object: support gcc function name mangling Fixes #189. Fixes #228. --- kpatch-build/create-diff-object.c | 62 ++++++++++++++++++++++++++++ test/integration/gcc-constprop.patch | 13 ++++++ test/integration/gcc-isra.patch | 12 ++++++ 3 files changed, 87 insertions(+) create mode 100644 test/integration/gcc-constprop.patch create mode 100644 test/integration/gcc-isra.patch diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index a59d8f9..c3a8144 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -221,6 +221,19 @@ struct symbol *find_symbol_by_name(struct list_head *list, const char *name) return NULL; } +struct symbol *find_symbol_by_name_prefix(struct list_head *list, + const char *name) +{ + struct symbol *sym; + int namelen = strlen(name); + + list_for_each_entry(sym, list, list) + if (!strncmp(sym->name, name, namelen)) + return sym; + + return NULL; +} + #define ALLOC_LINK(_new, _list) \ { \ (_new) = malloc(sizeof(*(_new))); \ @@ -678,6 +691,53 @@ void kpatch_check_program_headers(Elf *elf) DIFF_FATAL("ELF contains program header"); } +/* + * When gcc makes compiler optimizations which affect a function's calling + * interface, it mangles the function's name. For example, sysctl_print_dir is + * renamed to sysctl_print_dir.isra.2. The problem is that the trailing number + * is chosen arbitrarily, and the patched version of the function may end up + * with a different trailing number. Rename any mangled patched functions to + * match their original counterparts. + */ +void kpatch_rename_mangled_functions(struct kpatch_elf *orig, + struct kpatch_elf *patched) +{ + struct symbol *sym, *origsym; + char *prefix; + int i; + + list_for_each_entry(sym, &patched->symbols, list) { + if (sym->type != STT_FUNC) + continue; + + if (!strstr(sym->name, ".isra.") && + !strstr(sym->name, ".constprop.")) + continue; + + if (sym != sym->sec->sym) + ERROR("expected bundled section for %s\n", sym->name); + + prefix = malloc(strlen(sym->name) + 1); + for (i = 0; sym->name[i] != '.'; i++) + prefix[i] = sym->name[i]; + prefix[i] = 0; + + origsym = find_symbol_by_name_prefix(&orig->symbols, prefix); + free(prefix); + if (!origsym) + continue; + + if (!strcmp(sym->name, origsym->name)) + continue; + + log_debug("renaming %s to %s\n", sym->name, origsym->name); + sym->name = strdup(origsym->name); + sym->sec->name = strdup(origsym->sec->name); + if (sym->sec->rela) + sym->sec->rela->name = strdup(origsym->sec->rela->name); + } +} + void kpatch_correlate_elfs(struct kpatch_elf *kelf1, struct kpatch_elf *kelf2) { kpatch_correlate_sections(&kelf1->sections, &kelf2->sections); @@ -1970,6 +2030,8 @@ int main(int argc, char *argv[]) kpatch_check_program_headers(kelf_base->elf); kpatch_check_program_headers(kelf_patched->elf); + kpatch_rename_mangled_functions(kelf_base, kelf_patched); + kpatch_correlate_elfs(kelf_base, kelf_patched); /* * After this point, we don't care about kelf_base anymore. diff --git a/test/integration/gcc-constprop.patch b/test/integration/gcc-constprop.patch new file mode 100644 index 0000000..bec7cd8 --- /dev/null +++ b/test/integration/gcc-constprop.patch @@ -0,0 +1,13 @@ +Index: src/kernel/time/timekeeping.c +=================================================================== +--- src.orig/kernel/time/timekeeping.c ++++ src/kernel/time/timekeeping.c +@@ -480,6 +480,8 @@ void do_gettimeofday(struct timeval *tv) + { + struct timespec now; + ++ if (!tv) ++ return; + getnstimeofday(&now); + tv->tv_sec = now.tv_sec; + tv->tv_usec = now.tv_nsec/1000; diff --git a/test/integration/gcc-isra.patch b/test/integration/gcc-isra.patch new file mode 100644 index 0000000..a92364b --- /dev/null +++ b/test/integration/gcc-isra.patch @@ -0,0 +1,12 @@ +Index: src/fs/proc/proc_sysctl.c +=================================================================== +--- src.orig/fs/proc/proc_sysctl.c ++++ src/fs/proc/proc_sysctl.c +@@ -24,6 +24,7 @@ void proc_sys_poll_notify(struct ctl_tab + if (!poll) + return; + ++ printk("kpatch-test: testing gcc .isra function name mangling\n"); + atomic_inc(&poll->event); + wake_up_interruptible(&poll->wait); + } From 6be51b012ee31db622cc63d61ee75d882fbe1ab4 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 3 Jun 2014 12:16:51 -0500 Subject: [PATCH 2/2] fix review comments --- kpatch-build/create-diff-object.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index c3a8144..7fd3630 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -697,14 +697,13 @@ void kpatch_check_program_headers(Elf *elf) * renamed to sysctl_print_dir.isra.2. The problem is that the trailing number * is chosen arbitrarily, and the patched version of the function may end up * with a different trailing number. Rename any mangled patched functions to - * match their original counterparts. + * match their base counterparts. */ -void kpatch_rename_mangled_functions(struct kpatch_elf *orig, +void kpatch_rename_mangled_functions(struct kpatch_elf *base, struct kpatch_elf *patched) { - struct symbol *sym, *origsym; - char *prefix; - int i; + struct symbol *sym, *basesym; + char *prefix, *dot; list_for_each_entry(sym, &patched->symbols, list) { if (sym->type != STT_FUNC) @@ -717,24 +716,23 @@ void kpatch_rename_mangled_functions(struct kpatch_elf *orig, if (sym != sym->sec->sym) ERROR("expected bundled section for %s\n", sym->name); - prefix = malloc(strlen(sym->name) + 1); - for (i = 0; sym->name[i] != '.'; i++) - prefix[i] = sym->name[i]; - prefix[i] = 0; + prefix = strdup(sym->name); + dot = strchr(prefix, '.'); + *dot = '\0'; - origsym = find_symbol_by_name_prefix(&orig->symbols, prefix); + basesym = find_symbol_by_name_prefix(&base->symbols, prefix); free(prefix); - if (!origsym) + if (!basesym) continue; - if (!strcmp(sym->name, origsym->name)) + if (!strcmp(sym->name, basesym->name)) continue; - log_debug("renaming %s to %s\n", sym->name, origsym->name); - sym->name = strdup(origsym->name); - sym->sec->name = strdup(origsym->sec->name); + log_debug("renaming %s to %s\n", sym->name, basesym->name); + sym->name = strdup(basesym->name); + sym->sec->name = strdup(basesym->sec->name); if (sym->sec->rela) - sym->sec->rela->name = strdup(origsym->sec->rela->name); + sym->sec->rela->name = strdup(basesym->sec->rela->name); } }