From ea0470baa7fb5747523ddb938170004f84834b63 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 10 Aug 2021 13:55:32 -0700 Subject: [PATCH 1/3] create-diff-object: change kpatch_line_macro_change_only() return type to bool Make kpatch_line_macro_change_only()'s usage more clear by changing its return type to bool. Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 3b449fd..6c3d956 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -609,7 +609,7 @@ out: * 3) (optional) __warned.xxxxx static local rela * 4) warn_slowpath_* or __might_sleep or some other similar rela */ -static int kpatch_line_macro_change_only(struct section *sec) +static bool kpatch_line_macro_change_only(struct section *sec) { struct insn insn1, insn2; unsigned long start1, start2, size, offset, length; @@ -622,7 +622,7 @@ static int kpatch_line_macro_change_only(struct section *sec) sec->sh.sh_size != sec->twin->sh.sh_size || !sec->rela || sec->rela->status != SAME) - return 0; + return false; start1 = (unsigned long)sec->twin->data->d_buf; start2 = (unsigned long)sec->data->d_buf; @@ -639,7 +639,7 @@ static int kpatch_line_macro_change_only(struct section *sec) sec->name, offset); if (insn1.length != insn2.length) - return 0; + return false; if (!memcmp((void *)start1 + offset, (void *)start2 + offset, length)) @@ -650,7 +650,7 @@ static int kpatch_line_macro_change_only(struct section *sec) insn_get_opcode(&insn2); if (!(insn1.opcode.value == 0xba && insn2.opcode.value == 0xba) && !(insn1.opcode.value == 0xbe && insn2.opcode.value == 0xbe)) - return 0; + return false; /* * Verify zero or more string relas followed by a @@ -674,10 +674,10 @@ static int kpatch_line_macro_change_only(struct section *sec) found = 1; break; } - return 0; + return false; } if (!found) - return 0; + return false; lineonly = 1; } @@ -686,13 +686,13 @@ static int kpatch_line_macro_change_only(struct section *sec) ERROR("no instruction changes detected for changed section %s", sec->name); - return 1; + return true; } #elif __powerpc64__ #define PPC_INSTR_LEN 4 #define PPC_RA_OFFSET 16 -static int kpatch_line_macro_change_only(struct section *sec) +static bool kpatch_line_macro_change_only(struct section *sec) { unsigned long start1, start2, size, offset; unsigned int instr1, instr2; @@ -705,7 +705,7 @@ static int kpatch_line_macro_change_only(struct section *sec) sec->sh.sh_size != sec->twin->sh.sh_size || !sec->rela || sec->rela->status != SAME) - return 0; + return false; start1 = (unsigned long)sec->twin->data->d_buf; start2 = (unsigned long)sec->data->d_buf; @@ -720,7 +720,7 @@ static int kpatch_line_macro_change_only(struct section *sec) /* verify it's a load immediate to r5 */ if (!(instr1 == 0x38a0 && instr2 == 0x38a0)) - return 0; + return false; found = 0; list_for_each_entry(rela, &sec->rela->relas, list) { @@ -740,10 +740,10 @@ static int kpatch_line_macro_change_only(struct section *sec) found = 1; break; } - return 0; + return false; } if (!found) - return 0; + return false; lineonly = 1; } @@ -752,12 +752,12 @@ static int kpatch_line_macro_change_only(struct section *sec) ERROR("no instruction changes detected for changed section %s", sec->name); - return 1; + return true; } #else -static int kpatch_line_macro_change_only(struct section *sec) +static bool kpatch_line_macro_change_only(struct section *sec) { - return 0; + return false; } #endif From 6cf50a6fca8414a2414bcb1e87ec8fd81be806eb Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 10 Aug 2021 14:14:22 -0700 Subject: [PATCH 2/3] create-diff-object: Support CONFIG_PRINTK_INDEX, part 1 CONFIG_PRINTK_INDEX creates a static local struct variable named `_entry` for every call site to printk(). The initializer for that struct assigns the `__LINE__` macro to one of its fields. Similarly to the WARN macro's usage [1] of `__LINE__`, it causes problems because it results in the line number getting directly embedded in the struct. If a line is added or removed higher up in the source file, the `_entry` struct changes accordingly due to a change in the printk() call site line number. `_entry` is similar to other "special" static locals, in that we don't need to correlate the patched version with the original version. We can instead just ignore any changes to it. Any substantial (non-line-number) change to the `_entry` struct would be a second-order (dependent) effect of a first-order code change, which would be detected using other means. In that case the patched version of `_entry` will be included, due to being referenced by the changed function. Fixes: #1206 [1] See kpatch_line_macro_change_only() Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 1 + test/unit/objs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 6c3d956..92fb8e8 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -321,6 +321,7 @@ static bool is_special_static(struct symbol *sym) "__FUNCTION__", "_rs", "CSWTCH", + "_entry", NULL, }; char **var_name; diff --git a/test/unit/objs b/test/unit/objs index 7e8b18d..375794c 160000 --- a/test/unit/objs +++ b/test/unit/objs @@ -1 +1 @@ -Subproject commit 7e8b18d1b7dfef2394fb97668a4c568078c30c30 +Subproject commit 375794c2e5c6898643fea44f7ce354dc6404a4cc From 56471ffc7c0cdcb910217c828711786abcf831ea Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 10 Aug 2021 14:35:24 -0700 Subject: [PATCH 3/3] kpatch-build: Support CONFIG_PRINTK_INDEX, part 2 For each printk() call site, CONFIG_PRINTK_INDEX makes a static local struct named `_entry`, and then adds a pointer to it in the `.printk_index` section. When regenerating the `.printk_index` section for the patch module, we only need to include those entries which are referenced by included functions. Luckily this is a common pattern already used by several other "special" sections. Add `.printk_index` to the special section handling logic. Fixes: #1206 Signed-off-by: Josh Poimboeuf --- kpatch-build/create-diff-object.c | 19 +++++++++++++++++++ kpatch-build/kpatch-build | 16 +++++++++++++--- test/unit/objs | 2 +- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 92fb8e8..1ea587f 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -2004,6 +2004,21 @@ static int jump_table_group_size(struct kpatch_elf *kelf, int offset) return size; } +static int printk_index_group_size(struct kpatch_elf *kelf, int offset) +{ + static int size = 0; + char *str; + + if (!size) { + str = getenv("PRINTK_INDEX_STRUCT_SIZE"); + if (!str) + ERROR("PRINTK_INDEX_STRUCT_SIZE not set"); + size = atoi(str); + } + + return size; +} + #ifdef __x86_64__ static int parainstructions_group_size(struct kpatch_elf *kelf, int offset) { @@ -2149,6 +2164,10 @@ static struct special_section special_sections[] = { .name = "__jump_table", .group_size = jump_table_group_size, }, + { + .name = ".printk_index", + .group_size = printk_index_group_size, + }, #ifdef __x86_64__ { .name = ".smp_locks", diff --git a/kpatch-build/kpatch-build b/kpatch-build/kpatch-build index 186b725..3f3e708 100755 --- a/kpatch-build/kpatch-build +++ b/kpatch-build/kpatch-build @@ -329,6 +329,7 @@ clang_version_check() { find_special_section_data_ppc64le() { [[ "$CONFIG_JUMP_LABEL" -eq 0 ]] && AWK_OPTIONS="-vskip_j=1" + [[ "$CONFIG_PRINTK_INDEX" -eq 0 ]] && AWK_OPTIONS="$AWK_OPTIONS -vskip_i=1" SPECIAL_VARS="$(readelf -wi "$VMLINUX" | gawk --non-decimal-data ' @@ -339,21 +340,24 @@ find_special_section_data_ppc64le() { b == 0 && /DW_AT_name.* bug_entry[[:space:]]*$/ {b = 1; next} e == 0 && /DW_AT_name.* exception_table_entry[[:space:]]*$/ {e = 1; next} j == 0 && /DW_AT_name.* jump_entry[[:space:]]*$/ {j = 1; next} + i == 0 && /DW_AT_name.* pi_entry[[:space:]]*$/ {i = 1; next} # Reset state unless this abbrev describes the struct size f == 1 && !/DW_AT_byte_size/ { f = 0; next } b == 1 && !/DW_AT_byte_size/ { b = 0; next } e == 1 && !/DW_AT_byte_size/ { e = 0; next } j == 1 && !/DW_AT_byte_size/ { j = 0; next } + i == 1 && !/DW_AT_byte_size/ { i = 0; next } # Now that we know the size, stop parsing for it f == 1 {printf("export FIXUP_STRUCT_SIZE=%d\n", $4); f = 2} b == 1 {printf("export BUG_STRUCT_SIZE=%d\n", $4); b = 2} e == 1 {printf("export EX_STRUCT_SIZE=%d\n", $4); e = 2} j == 1 {printf("export JUMP_STRUCT_SIZE=%d\n", $4); j = 2} + i == 1 {printf("export PRINTK_INDEX_STRUCT_SIZE=%d\n", $4); i = 2} # Bail out once we have everything - f == 2 && b == 2 && e == 2 && (j == 2 || skip_j) {exit}')" + f == 2 && b == 2 && e == 2 && (j == 2 || skip_j) && (i == 2 || skip_i) {exit}')" [[ -n "$SPECIAL_VARS" ]] && eval "$SPECIAL_VARS" @@ -361,6 +365,7 @@ find_special_section_data_ppc64le() { [[ -z "$BUG_STRUCT_SIZE" ]] && die "can't find special struct bug_entry size" [[ -z "$EX_STRUCT_SIZE" ]] && die "can't find special struct exception_table_entry size" [[ -z "$JUMP_STRUCT_SIZE" && "$CONFIG_JUMP_LABEL" -ne 0 ]] && die "can't find special struct jump_entry size" + [[ -z "$PRINTK_INDEX_STRUCT_SIZE" && "$CONFIG_PRINTK_INDEX" -ne 0 ]] && die "can't find special struct pi_entry size" return } @@ -374,13 +379,14 @@ find_special_section_data() { [[ "$CONFIG_PARAVIRT" -eq 0 ]] && AWK_OPTIONS="-vskip_p=1" [[ "$CONFIG_UNWINDER_ORC" -eq 0 ]] && AWK_OPTIONS="$AWK_OPTIONS -vskip_o=1" [[ "$CONFIG_JUMP_LABEL" -eq 0 ]] && AWK_OPTIONS="$AWK_OPTIONS -vskip_j=1" + [[ "$CONFIG_PRINTK_INDEX" -eq 0 ]] && AWK_OPTIONS="$AWK_OPTIONS -vskip_i=1" ! kernel_version_gte 5.10.0 && AWK_OPTIONS="$AWK_OPTIONS -vskip_s=1" # If $AWK_OPTIONS are blank gawk would treat "" as a blank script # shellcheck disable=SC2086 SPECIAL_VARS="$(readelf -wi "$VMLINUX" | gawk --non-decimal-data $AWK_OPTIONS ' - BEGIN { a = b = p = e = o = j = s = 0 } + BEGIN { a = b = p = e = o = j = s = i = 0 } # Set state if name matches a == 0 && /DW_AT_name.* alt_instr[[:space:]]*$/ {a = 1; next} @@ -390,6 +396,7 @@ find_special_section_data() { o == 0 && /DW_AT_name.* orc_entry[[:space:]]*$/ {o = 1; next} j == 0 && /DW_AT_name.* jump_entry[[:space:]]*$/ {j = 1; next} s == 0 && /DW_AT_name.* static_call_site[[:space:]]*$/ {s = 1; next} + i == 0 && /DW_AT_name.* pi_entry[[:space:]]*$/ {i = 1; next} # Reset state unless this abbrev describes the struct size a == 1 && !/DW_AT_byte_size/ { a = 0; next } @@ -399,6 +406,7 @@ find_special_section_data() { o == 1 && !/DW_AT_byte_size/ { o = 0; next } j == 1 && !/DW_AT_byte_size/ { j = 0; next } s == 1 && !/DW_AT_byte_size/ { s = 0; next } + i == 1 && !/DW_AT_byte_size/ { i = 0; next } # Now that we know the size, stop parsing for it a == 1 {printf("export ALT_STRUCT_SIZE=%d\n", $4); a = 2} @@ -408,9 +416,10 @@ find_special_section_data() { o == 1 {printf("export ORC_STRUCT_SIZE=%d\n", $4); o = 2} j == 1 {printf("export JUMP_STRUCT_SIZE=%d\n", $4); j = 2} s == 1 {printf("export STATIC_CALL_STRUCT_SIZE=%d\n", $4); s = 2} + i == 1 {printf("export PRINTK_INDEX_STRUCT_SIZE=%d\n", $4); i = 2} # Bail out once we have everything - a == 2 && b == 2 && (p == 2 || skip_p) && e == 2 && (o == 2 || skip_o) && (j == 2 || skip_j) && (s == 2 || skip_s) {exit}')" + a == 2 && b == 2 && (p == 2 || skip_p) && e == 2 && (o == 2 || skip_o) && (j == 2 || skip_j) && (s == 2 || skip_s) && (i == 2 || skip_i) {exit}')" [[ -n "$SPECIAL_VARS" ]] && eval "$SPECIAL_VARS" @@ -420,6 +429,7 @@ find_special_section_data() { [[ -z "$PARA_STRUCT_SIZE" && "$CONFIG_PARAVIRT" -ne 0 ]] && die "can't find special struct paravirt_patch_site size" [[ -z "$ORC_STRUCT_SIZE" && "$CONFIG_UNWINDER_ORC" -ne 0 ]] && die "can't find special struct orc_entry size" [[ -z "$JUMP_STRUCT_SIZE" && "$CONFIG_JUMP_LABEL" -ne 0 ]] && die "can't find special struct jump_entry size" + [[ -z "$PRINTK_INDEX_STRUCT_SIZE" && "$CONFIG_PRINTK_INDEX" -ne 0 ]] && die "can't find special struct pi_entry size" [[ -z "$STATIC_CALL_STRUCT_SIZE" ]] && kernel_version_gte 5.10.0 && die "can't find special struct static_call_site size" return diff --git a/test/unit/objs b/test/unit/objs index 375794c..32ab24d 160000 --- a/test/unit/objs +++ b/test/unit/objs @@ -1 +1 @@ -Subproject commit 375794c2e5c6898643fea44f7ce354dc6404a4cc +Subproject commit 32ab24dc590c955f50f794afc0ccc2d8d9a92aa2