From 017c5e6395216b88d2780555360983b8409dc772 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 3 Sep 2014 09:55:40 -0500 Subject: [PATCH 1/4] allow multiple references to same static local var This fixes a logic bug in the static local variable code where we don't allow multiple relocation references to the same static local variable symbol. --- 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 fb38fd2..efee973 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -942,7 +942,7 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, continue; if (strncmp(rela->sym->name, sym->name, prefixlen)) continue; - if (basesym) + if (basesym && basesym != rela->sym) ERROR("found two static local variables matching %s in orig %s", sym->name, sec->name); From 8ac338aac411aa05d62cb15208e52eb21ad7f70d Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 3 Sep 2014 09:57:57 -0500 Subject: [PATCH 2/4] support renaming of unbundled static locals WARN_ON_ONCE places the __warned static local variable in the .data.unlikely section, so it's not bundled (i.e. ignored by the -fdata-sections gcc flag). There's no reason why we can't rename unbundled symbols, so add support for them. Fixes #394. --- kpatch-build/create-diff-object.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index efee973..a22e7b2 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -867,7 +867,7 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, struct symbol *sym, *basesym; struct section *tmpsec, *sec; struct rela *rela; - int prefixlen; + int prefixlen, bundled1, bundled2; char *dot; list_for_each_entry(sym, &patched->symbols, list) { @@ -951,19 +951,25 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, if (!basesym) continue; - if (sym != sym->sec->sym) - ERROR("expected bundled section for %s", sym->name); - if (basesym != basesym->sec->sym) - ERROR("expected bundled section for %s",basesym->name); + bundled1 = sym == sym->sec->sym; + bundled2 = basesym == basesym->sec->sym; + if (bundled1 != bundled2) + ERROR("bundle mismatch for symbol %s", sym->name); + if (!bundled1 && sym->sec->twin != basesym->sec) + ERROR("sections %s and %s aren't correlated", + sym->sec->name, basesym->sec->name); log_debug("renaming and correlating %s to %s\n", sym->name, basesym->name); sym->name = strdup(basesym->name); sym->twin = basesym; basesym->twin = sym; - sym->sec->twin = basesym->sec; - basesym->sec->twin = sym->sec; sym->status = basesym->status = SAME; + + if (bundled1) { + sym->sec->twin = basesym->sec; + basesym->sec->twin = sym->sec; + } } } From aca9b525678f3357eedc8aade0cd7f72de77e6a7 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 3 Sep 2014 10:03:40 -0500 Subject: [PATCH 3/4] add test case for issue #394. --- test/integration/gcc-static-local-var-2.patch | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/integration/gcc-static-local-var-2.patch diff --git a/test/integration/gcc-static-local-var-2.patch b/test/integration/gcc-static-local-var-2.patch new file mode 100644 index 0000000..d913a26 --- /dev/null +++ b/test/integration/gcc-static-local-var-2.patch @@ -0,0 +1,22 @@ +Index: src/mm/mmap.c +=================================================================== +--- src.orig/mm/mmap.c ++++ src/mm/mmap.c +@@ -1493,6 +1493,7 @@ static inline int accountable_mapping(st + return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE; + } + ++#include "kpatch-macros.h" + unsigned long mmap_region(struct file *file, unsigned long addr, + unsigned long len, vm_flags_t vm_flags, unsigned long pgoff) + { +@@ -1502,6 +1503,9 @@ unsigned long mmap_region(struct file *f + struct rb_node **rb_link, *rb_parent; + unsigned long charged = 0; + ++ if (!jiffies) ++ printk("kpatch mmap foo\n"); ++ + /* Check against address space limit. */ + if (!may_expand_vm(mm, len >> PAGE_SHIFT)) { + unsigned long nr_pages; From 02fcfa506b43aedfe41120b56efed9912010b9d5 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 3 Sep 2014 13:10:48 -0500 Subject: [PATCH 4/4] code review fixes Rename bundled1 to bundled and bundled2 to basebundled. --- kpatch-build/create-diff-object.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index a22e7b2..7b1e9e1 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -867,7 +867,7 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, struct symbol *sym, *basesym; struct section *tmpsec, *sec; struct rela *rela; - int prefixlen, bundled1, bundled2; + int prefixlen, bundled, basebundled; char *dot; list_for_each_entry(sym, &patched->symbols, list) { @@ -951,11 +951,11 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, if (!basesym) continue; - bundled1 = sym == sym->sec->sym; - bundled2 = basesym == basesym->sec->sym; - if (bundled1 != bundled2) + bundled = sym == sym->sec->sym; + basebundled = basesym == basesym->sec->sym; + if (bundled != basebundled) ERROR("bundle mismatch for symbol %s", sym->name); - if (!bundled1 && sym->sec->twin != basesym->sec) + if (!bundled && sym->sec->twin != basesym->sec) ERROR("sections %s and %s aren't correlated", sym->sec->name, basesym->sec->name); @@ -966,7 +966,7 @@ void kpatch_correlate_static_local_variables(struct kpatch_elf *base, basesym->twin = sym; sym->status = basesym->status = SAME; - if (bundled1) { + if (bundled) { sym->sec->twin = basesym->sec; basesym->sec->twin = sym->sec; }