From 358743e6d8748510f4c9a71511d7ceea7c72f7aa Mon Sep 17 00:00:00 2001
From: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Mon, 21 Nov 2022 19:23:07 -0800
Subject: [PATCH] v0.9.7 backport: MR!1315 ("Static call fixes")
commit 87ad96760a3af0db294d44865dfa1703f57f5595
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Mon Nov 21 19:23:07 2022 -0800
create-diff-object: fix s390 special_section initializer spacing
Align the s390 special_section initializers to improve readability and
for consistency with the rest.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
commit 56bd8c4d0da1634f8549e7269f77a53e9d936a57
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Mon Nov 21 19:27:23 2022 -0800
create-diff-object: refactor jump label filtering
Convert the hard-coded should_keep_jump_label() to a proper callback,
since static calls will need a similar filter.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
commit f83218ad12a2d9e20d99d379c78974a576aa558c
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Mon Nov 21 19:29:53 2022 -0800
create-diff-object: detect unsupported static calls
Similar to jump labels, static calls aren't supported when the static
call key was originally defined in a module rather than in vmlinux.
Detect those cases and either remove them (in the case of tracepoints)
or error out.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
commit ab2397c03e31f0f697aa8bf943d70b4e5a7def54
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Mon Nov 21 19:41:30 2022 -0800
kpatch-macros: add KPATCH_STATIC_CALL()
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
commit 92c178b6a30a827c48db46ff4238501ec406a28e
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Tue Nov 22 12:53:09 2022 -0800
create-diff-object: use errx() instead of err()
Otherwise on recent distros it appends the errno to the error message,
like:
create-diff-object: ERROR: x86.o: kpatch_regenerate_special_section: 2633: Found 1 unsupported static call(s) in the patched code. Use KPATCH_STATIC_CALL() instead.: Success
which is not what we want in most cases.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Yannick Cote <ycote@redhat.com>
---
kmod/patch/kpatch-macros.h | 11 +
kpatch-build/create-diff-object.c | 328 ++++++++++++++++++------------
kpatch-build/log.h | 2 +-
3 files changed, 211 insertions(+), 130 deletions(-)
diff --git a/kmod/patch/kpatch-macros.h b/kmod/patch/kpatch-macros.h
index 8e09702ea001..b797838849ca 100644
--- a/kmod/patch/kpatch-macros.h
+++ b/kmod/patch/kpatch-macros.h
@@ -141,4 +141,15 @@ struct kpatch_post_unpatch_callback {
printk(_fmt, ## __VA_ARGS__); \
})
+/*
+ * KPATCH_STATIC_CALL macro
+ *
+ * Replace usages of static_call() with this macro, when create-diff-object
+ * recommends it due to the original static call key living in a module.
+ *
+ * This converts the static call to a regular indirect call.
+ */
+#define KPATCH_STATIC_CALL(name) \
+ ((typeof(STATIC_CALL_TRAMP(name))*)(STATIC_CALL_KEY(name).func))
+
#endif /* __KPATCH_MACROS_H_ */
diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c
index 7106b67cfd25..ddaa9b44f11e 100644
--- a/kpatch-build/create-diff-object.c
+++ b/kpatch-build/create-diff-object.c
@@ -56,7 +56,7 @@
#define DIFF_FATAL(format, ...) \
({ \
fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
- err(EXIT_STATUS_DIFF_FATAL, "unreconcilable difference"); \
+ errx(EXIT_STATUS_DIFF_FATAL, "unreconcilable difference"); \
})
char *childobj;
@@ -71,6 +71,8 @@ enum loglevel loglevel = NORMAL;
bool KLP_ARCH;
+int jump_label_errors, static_call_errors;
+
/*******************
* Data structures
* ****************/
@@ -78,6 +80,9 @@ struct special_section {
char *name;
enum architecture arch;
int (*group_size)(struct kpatch_elf *kelf, int offset);
+ bool (*group_filter)(struct lookup_table *lookup,
+ struct section *relasec, unsigned int offset,
+ unsigned int size);
};
/*************
@@ -2215,6 +2220,169 @@ static int fixup_group_size(struct kpatch_elf *kelf, int offset)
return (int)(rela->addend - offset);
}
+static bool jump_table_group_filter(struct lookup_table *lookup,
+ struct section *relasec,
+ unsigned int group_offset,
+ unsigned int group_size)
+{
+ struct rela *code = NULL, *key = NULL, *rela;
+ bool tracepoint = false, dynamic_debug = false;
+ struct lookup_result symbol;
+ int i = 0;
+
+ /*
+ * Here we hard-code knowledge about the contents of the jump_entry
+ * struct. It has three fields: code, target, and key. Each field has
+ * a relocation associated with it.
+ */
+ list_for_each_entry(rela, &relasec->relas, list) {
+ if (rela->offset >= group_offset &&
+ rela->offset < group_offset + group_size) {
+ if (i == 0)
+ code = rela;
+ else if (i == 2)
+ key = rela;
+ i++;
+ }
+ }
+
+ if (i != 3 || !key || !code)
+ ERROR("BUG: __jump_table has an unexpected format");
+
+ if (!strncmp(key->sym->name, "__tracepoint_", 13))
+ tracepoint = true;
+
+ if (is_dynamic_debug_symbol(key->sym))
+ dynamic_debug = true;
+
+ if (KLP_ARCH) {
+ /*
+ * On older kernels (with .klp.arch support), jump labels
+ * aren't supported at all. Error out when they occur in a
+ * replacement function, with the exception of tracepoints and
+ * dynamic debug printks. An inert tracepoint or printk is
+ * harmless enough, but a broken jump label can cause
+ * unexpected behavior.
+ */
+ if (tracepoint || dynamic_debug)
+ return false;
+
+ /*
+ * This will be upgraded to an error after all jump labels have
+ * been reported.
+ */
+ log_normal("Found a jump label at %s()+0x%lx, using key %s. Jump labels aren't supported with this kernel. Use static_key_enabled() instead.\n",
+ code->sym->name, code->addend, key->sym->name);
+ jump_label_errors++;
+ return false;
+ }
+
+ /*
+ * On newer (5.8+) kernels, jump labels are supported in the case where
+ * the corresponding static key lives in vmlinux. That's because such
+ * kernels apply vmlinux-specific .klp.rela sections at the same time
+ * (in the klp module load) as normal relas, before jump label init.
+ * On the other hand, jump labels based on static keys which are
+ * defined in modules aren't supported, because late module patching
+ * can result in the klp relas getting applied *after* the klp module's
+ * jump label init.
+ */
+
+ if (lookup_symbol(lookup, key->sym, &symbol) &&
+ strcmp(symbol.objname, "vmlinux")) {
+
+ /* The static key lives in a module -- not supported */
+
+ /* Inert tracepoints and dynamic debug printks are harmless */
+ if (tracepoint || dynamic_debug)
+ return false;
+
+ /*
+ * This will be upgraded to an error after all jump label
+ * errors have been reported.
+ */
+ log_normal("Found a jump label at %s()+0x%lx, using key %s, which is defined in a module. Use static_key_enabled() instead.\n",
+ code->sym->name, code->addend, key->sym->name);
+ jump_label_errors++;
+ return false;
+ }
+
+ /* The static key lives in vmlinux or the patch module itself */
+
+ /*
+ * If the jump label key lives in the '__dyndbg' section, make sure
+ * the section gets included, because we don't use klp relocs for
+ * dynamic debug symbols. For an example of such a key, see
+ * DYNAMIC_DEBUG_BRANCH().
+ */
+ if (dynamic_debug)
+ kpatch_include_symbol(key->sym);
+
+ return true;
+}
+
+static bool static_call_sites_group_filter(struct lookup_table *lookup,
+ struct section *relasec,
+ unsigned int group_offset,
+ unsigned int group_size)
+{
+ struct rela *code = NULL, *key = NULL, *rela;
+ bool tracepoint = false;
+ struct lookup_result symbol;
+ int i = 0;
+
+ /*
+ * Here we hard-code knowledge about the contents of the jump_entry
+ * struct. It has three fields: code, target, and key. Each field has
+ * a relocation associated with it.
+ */
+ list_for_each_entry(rela, &relasec->relas, list) {
+ if (rela->offset >= group_offset &&
+ rela->offset < group_offset + group_size) {
+ if (i == 0)
+ code = rela;
+ else if (i == 1)
+ key = rela;
+ i++;
+ }
+ }
+
+ if (i != 2 || !key || !code)
+ ERROR("BUG: .static_call_sites has an unexpected format");
+
+ if (!strncmp(key->sym->name, "__SCK__tp_func_", 15))
+ tracepoint = true;
+
+ /*
+ * Static calls are only supported in the case where the corresponding
+ * static call key lives in vmlinux (see explanation in
+ * jump_table_group_filter).
+ */
+
+ if (lookup_symbol(lookup, key->sym, &symbol) &&
+ strcmp(symbol.objname, "vmlinux")) {
+
+ /* The key lives in a module -- not supported */
+
+ /* Inert tracepoints are harmless */
+ if (tracepoint)
+ return false;
+
+ /*
+ * This will be upgraded to an error after all static call
+ * errors have been reported.
+ */
+ log_normal("Found a static call at %s()+0x%lx, using key %s, which is defined in a module. Use KPATCH_STATIC_CALL() instead.\n",
+ code->sym->name, code->addend, key->sym->name);
+ static_call_errors++;
+ return false;
+ }
+
+ /* The key lives in vmlinux or the patch module itself */
+ return true;
+}
+
+
static struct special_section special_sections[] = {
{
.name = "__bug_table",
@@ -2235,6 +2403,7 @@ static struct special_section special_sections[] = {
.name = "__jump_table",
.arch = X86_64 | PPC64 | S390,
.group_size = jump_table_group_size,
+ .group_filter = jump_table_group_filter,
},
{
.name = ".printk_index",
@@ -2260,6 +2429,7 @@ static struct special_section special_sections[] = {
.name = ".static_call_sites",
.arch = X86_64,
.group_size = static_call_sites_group_size,
+ .group_filter = static_call_sites_group_filter,
},
{
.name = ".retpoline_sites",
@@ -2297,138 +2467,36 @@ static struct special_section special_sections[] = {
.group_size = fixup_barrier_nospec_group_size,
},
{
- .name = ".s390_return_mem",
- .arch = S390,
- .group_size = s390_expolines_group_size,
+ .name = ".s390_return_mem",
+ .arch = S390,
+ .group_size = s390_expolines_group_size,
},
{
- .name = ".s390_return_reg",
- .arch = S390,
- .group_size = s390_expolines_group_size,
+ .name = ".s390_return_reg",
+ .arch = S390,
+ .group_size = s390_expolines_group_size,
},
{
- .name = ".s390_indirect_call",
- .arch = S390,
- .group_size = s390_expolines_group_size,
+ .name = ".s390_indirect_call",
+ .arch = S390,
+ .group_size = s390_expolines_group_size,
},
{
- .name = ".s390_indirect_branches",
- .arch = S390,
- .group_size = s390_expolines_group_size,
+ .name = ".s390_indirect_branches",
+ .arch = S390,
+ .group_size = s390_expolines_group_size,
},
{
- .name = ".s390_indirect_jump",
- .arch = S390,
- .group_size = s390_expolines_group_size,
+ .name = ".s390_indirect_jump",
+ .arch = S390,
+ .group_size = s390_expolines_group_size,
},
{},
};
-static bool should_keep_jump_label(struct lookup_table *lookup,
- struct section *relasec,
- unsigned int group_offset,
- unsigned int group_size,
- int *jump_labels_found)
-{
- struct rela *code = NULL, *key = NULL, *rela;
- bool tracepoint = false, dynamic_debug = false;
- struct lookup_result symbol;
- int i = 0;
-
- /*
- * Here we hard-code knowledge about the contents of the jump_entry
- * struct. It has three fields: code, target, and key. Each field has
- * a relocation associated with it.
- */
- list_for_each_entry(rela, &relasec->relas, list) {
- if (rela->offset >= group_offset &&
- rela->offset < group_offset + group_size) {
- if (i == 0)
- code = rela;
- else if (i == 2)
- key = rela;
- i++;
- }
- }
-
- if (i != 3 || !key || !code)
- ERROR("BUG: __jump_table has an unexpected format");
-
- if (!strncmp(key->sym->name, "__tracepoint_", 13))
- tracepoint = true;
-
- if (is_dynamic_debug_symbol(key->sym))
- dynamic_debug = true;
-
- if (KLP_ARCH) {
- /*
- * On older kernels (with .klp.arch support), jump labels
- * aren't supported at all. Error out when they occur in a
- * replacement function, with the exception of tracepoints and
- * dynamic debug printks. An inert tracepoint or printk is
- * harmless enough, but a broken jump label can cause
- * unexpected behavior.
- */
- if (tracepoint || dynamic_debug)
- return false;
-
- /*
- * This will be upgraded to an error after all jump labels have
- * been reported.
- */
- log_normal("Found a jump label at %s()+0x%lx, using key %s. Jump labels aren't supported with this kernel. Use static_key_enabled() instead.\n",
- code->sym->name, code->addend, key->sym->name);
- (*jump_labels_found)++;
- return false;
- }
-
- /*
- * On newer (5.8+) kernels, jump labels are supported in the case where
- * the corresponding static key lives in vmlinux. That's because such
- * kernels apply vmlinux-specific .klp.rela sections at the same time
- * (in the klp module load) as normal relas, before jump label init.
- * On the other hand, jump labels based on static keys which are
- * defined in modules aren't supported, because late module patching
- * can result in the klp relas getting applied *after* the klp module's
- * jump label init.
- */
-
- if (lookup_symbol(lookup, key->sym, &symbol) &&
- strcmp(symbol.objname, "vmlinux")) {
-
- /* The static key lives in a module -- not supported */
-
- /* Inert tracepoints and dynamic debug printks are harmless */
- if (tracepoint || dynamic_debug)
- return false;
-
- /*
- * This will be upgraded to an error after all jump labels have
- * been reported.
- */
- log_normal("Found a jump label at %s()+0x%lx, using key %s, which is defined in a module. Use static_key_enabled() instead.\n",
- code->sym->name, code->addend, key->sym->name);
- (*jump_labels_found)++;
- return false;
- }
-
- /* The static key lives in vmlinux or the patch module itself */
-
- /*
- * If the jump label key lives in the '__dyndbg' section, make sure
- * the section gets included, because we don't use klp relocs for
- * dynamic debug symbols. For an example of such a key, see
- * DYNAMIC_DEBUG_BRANCH().
- */
- if (dynamic_debug)
- kpatch_include_symbol(key->sym);
-
- return true;
-}
-
static bool should_keep_rela_group(struct lookup_table *lookup,
struct section *relasec, unsigned int offset,
- unsigned int size, int *jump_labels_found)
+ unsigned int size)
{
struct rela *rela;
bool found = false;
@@ -2448,10 +2516,6 @@ static bool should_keep_rela_group(struct lookup_table *lookup,
if (!found)
return false;
- if (!strcmp(relasec->name, ".rela__jump_table"))
- return should_keep_jump_label(lookup, relasec, offset, size,
- jump_labels_found);
-
return true;
}
@@ -2488,7 +2552,6 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf,
struct rela *rela, *safe;
char *src, *dest;
unsigned int group_size, src_offset, dest_offset;
- int jump_labels_found = 0;
LIST_HEAD(newrelas);
@@ -2523,8 +2586,11 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf,
if (src_offset + group_size > relasec->base->sh.sh_size)
group_size = (unsigned int)(relasec->base->sh.sh_size - src_offset);
- if (!should_keep_rela_group(lookup, relasec, src_offset, group_size,
- &jump_labels_found))
+ if (!should_keep_rela_group(lookup, relasec, src_offset, group_size))
+ continue;
+
+ if (special->group_filter &&
+ !special->group_filter(lookup, relasec, src_offset, group_size))
continue;
/*
@@ -2557,9 +2623,13 @@ static void kpatch_regenerate_special_section(struct kpatch_elf *kelf,
dest_offset += group_size;
}
- if (jump_labels_found)
- ERROR("Found %d jump label(s) in the patched code. Jump labels aren't currently supported. Use static_key_enabled() instead.",
- jump_labels_found);
+ if (jump_label_errors)
+ ERROR("Found %d unsupported jump label(s) in the patched code. Use static_key_enabled() instead.",
+ jump_label_errors);
+
+ if (static_call_errors)
+ ERROR("Found %d unsupported static call(s) in the patched code. Use KPATCH_STATIC_CALL() instead.",
+ static_call_errors);
if (!dest_offset) {
/* no changed or global functions referenced */
diff --git a/kpatch-build/log.h b/kpatch-build/log.h
index eefa0fce7b08..dbdc212713e1 100644
--- a/kpatch-build/log.h
+++ b/kpatch-build/log.h
@@ -9,7 +9,7 @@ extern enum loglevel loglevel;
extern char *childobj;
#define ERROR(format, ...) \
- err(EXIT_STATUS_ERROR, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
+ errx(EXIT_STATUS_ERROR, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__, __LINE__, ##__VA_ARGS__)
#define log_debug(format, ...) log(DEBUG, format, ##__VA_ARGS__)
#define log_normal(format, ...) log(NORMAL, "%s: " format, childobj, ##__VA_ARGS__)
--
2.38.1