From 09262d4d674d6c81ef4c1ef08b60b2ddfe7b4e89 Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 21 Apr 2014 17:25:28 -0500 Subject: [PATCH 1/5] support .bss.* bundling Allow bundling of .bss.* sections that are the result of -fdata-sections so that rela sections referencing data in bss sections by section symbol can be replaced with the object symbol so it can be linked to the existing data object in the kernel. Signed-off-by: Seth Jennings --- kpatch-build/create-diff-object.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index f4dd7eb..de2b966 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -336,6 +336,11 @@ int is_bundleable(struct symbol *sym) !strcmp(sym->sec->name + 6, sym->name)) return 1; + if (sym->type == STT_OBJECT && + !strncmp(sym->sec->name, ".bss.",5) && + !strcmp(sym->sec->name + 5, sym->name)) + return 1; + return 0; } From 3753f06de40d433d769e68bbd803fccadd7c0157 Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 21 Apr 2014 17:28:08 -0500 Subject: [PATCH 2/5] use d_size instead of sh_size The section header size is calculated at output time by libelf and we use it as a read-only value from read files. With the next patch we are changing the size of the .rela__bug_table section. Lets use d_size instead since it is the value that tells libelf how to calculate sh_size at output time. Signed-off-by: Seth Jennings --- 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 de2b966..c151c94 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -979,7 +979,7 @@ void kpatch_create_rela_section(struct section *sec, int link) size_t size; /* create new rela data buffer */ - size = sec->sh.sh_size; + size = sec->data->d_size; buf = malloc(size); if (!buf) ERROR("malloc"); From 7cfcce1ed6a018879fb3358cddd0a4183370b419 Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 21 Apr 2014 17:33:09 -0500 Subject: [PATCH 3/5] add bug table support This commit adds a new function to properly handle the bug table. It works by going through .rela__bug_table, after the changed function symbols have already been marked, and rewrites the section including only the relocations pertaining to bug entries for changed functions. The __bug_table section itself is not modified resulting in "blank" bug entries: ones whose IP and filename pointers will not be relocated and, therefore, will be zero. While a waste of space, it simplifies the code not to remove these blank entries. They do no harm. Signed-off-by: Seth Jennings --- kpatch-build/create-diff-object.c | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index c151c94..1ee980d 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -971,6 +971,63 @@ void kpatch_write_inventory_file(struct kpatch_elf *kelf, char *outfile) fclose(out); } +void kpatch_regenerate_bug_table(struct kpatch_elf *kelf) +{ + struct section *sec; + struct table table; + struct rela *rela, *dstrela; + int i, nr = 0, copynext = 0; + + sec = find_section_by_name(&kelf->sections, ".rela__bug_table"); + if (!sec || sec->status == SAME) + return; + + /* alloc buffer of original size (probably won't use it all) */ + alloc_table(&table, sizeof(struct rela), sec->relas.nr); + dstrela = table.data; + + for_each_rela(i, rela, &sec->relas) { + if (i % 2) { /* filename reloc */ + if (!copynext) + continue; + rela->sym->include = 1; + rela->sym->sec->include = 1; + *dstrela++ = *rela; + nr++; + copynext = 0; + } + else if (rela->sym->sec->status != SAME) { /* IP reloc */ + log_debug("new/changed symbol %s found in bug table\n", + rela->sym->name); + /* copy BOTH relocs for this bug_entry */ + *dstrela++ = *rela; + nr++; + /* tell the next loop to copy the filename reloc */ + copynext = 1; + } + } + + if (!nr) { + /* no changed functions references by bug table */ + sec->status = SAME; + sec->base->status = SAME; + return; + } + + /* overwrite with new relas table */ + table.nr = nr; + sec->relas = table; + sec->include = 1; + sec->base->include = 1; + /* + * Adjust d_size but not d_buf. d_buf is overwritten in + * kpatch_create_rela_section() from the relas table. No + * point in regen'ing the buffer here just to be discarded + * later. + */ + sec->data->d_size = sec->sh.sh_entsize * nr; +} + void kpatch_create_rela_section(struct section *sec, int link) { struct rela *rela; @@ -1323,6 +1380,7 @@ int main(int argc, char *argv[]) * in vmlinux can be linked to. */ kpatch_replace_sections_syms(kelf_patched); + kpatch_regenerate_bug_table(kelf_patched); kpatch_include_standard_sections(kelf_patched); if (!kpatch_include_changed_functions(kelf_patched)) { From ab07805166d7964cfd6945b96a2f2a00042d9232 Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Mon, 21 Apr 2014 17:39:14 -0500 Subject: [PATCH 4/5] add info to expected rela sym error While debugging the code for the bug table logic, I found it useful to know which rela section and entry the error occurred on. Signed-off-by: Seth Jennings --- 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 1ee980d..2697075 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -1045,7 +1045,8 @@ void kpatch_create_rela_section(struct section *sec, int link) /* reindex and copy into buffer */ for_each_rela(i, rela, &sec->relas) { if (!rela->sym || !rela->sym->twino) - ERROR("expected rela symbol"); + ERROR("expected rela symbol in rela section %s entry %d", + sec->name, i); symndx = rela->sym->twino->index; type = GELF_R_TYPE(rela->rela.r_info); rela->rela.r_info = GELF_R_INFO(symndx, type); From 47d4109f7e414df4254118152b6110d826f6e30d Mon Sep 17 00:00:00 2001 From: Seth Jennings Date: Tue, 22 Apr 2014 11:01:01 -0500 Subject: [PATCH 5/5] fixup review comments Signed-off-by: Seth Jennings --- kpatch-build/create-diff-object.c | 33 +++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 2697075..4788811 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -971,7 +971,32 @@ void kpatch_write_inventory_file(struct kpatch_elf *kelf, char *outfile) fclose(out); } -void kpatch_regenerate_bug_table(struct kpatch_elf *kelf) +/* + * The format of section __bug_table is a table of struct bug_entry. Each + * bug_entry has three fields: + * - relocated address of instruction pointer at BUG + * - relocated address of string with filename + * - line number of the BUG + * + * Therefore, .rela__bug_table has two relocations per entry. The first + * relocation is that of the instruction pointer at the BUG. The second is the + * pointer to the filename string in .rodata.str1.1. These two related + * relocations we will call a "pair". + * + * This function goes through .rela__bug_table and finds pairs the refer to + * functions that have been marked as changed. If one is found, that pair is + * copied into the new version of the .rela__bug_table section. If no pairs + * are found, the bug table (both the __bug_table and .rela__bug_table + * sections) are considered unchanged and not copied into the final output. + * + * The __bug_table section is not modified and therefore will contains "blank" + * bug_entry slots i.e. ones that do not get relocated and therefore the IP + * fields are zero. While this wastes space, it doesn't hurt anything and + * keeps the code cleaner by not having to regenerate the __bug_table section + * as well. + */ + +void kpatch_regenerate_bug_table_rela_section(struct kpatch_elf *kelf) { struct section *sec; struct table table; @@ -979,7 +1004,7 @@ void kpatch_regenerate_bug_table(struct kpatch_elf *kelf) int i, nr = 0, copynext = 0; sec = find_section_by_name(&kelf->sections, ".rela__bug_table"); - if (!sec || sec->status == SAME) + if (!sec) return; /* alloc buffer of original size (probably won't use it all) */ @@ -1008,7 +1033,7 @@ void kpatch_regenerate_bug_table(struct kpatch_elf *kelf) } if (!nr) { - /* no changed functions references by bug table */ + /* no changed functions referenced by bug table */ sec->status = SAME; sec->base->status = SAME; return; @@ -1381,7 +1406,7 @@ int main(int argc, char *argv[]) * in vmlinux can be linked to. */ kpatch_replace_sections_syms(kelf_patched); - kpatch_regenerate_bug_table(kelf_patched); + kpatch_regenerate_bug_table_rela_section(kelf_patched); kpatch_include_standard_sections(kelf_patched); if (!kpatch_include_changed_functions(kelf_patched)) {