hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I9UQ7I
--------------------------------
There is case that 'kernel_kexec' has multiple fentrys, and when CONFIG_LIVEPATCH_ISOLATE_KPROBE enable, livepatch want to find the fentry at function start, however it actually find a wrong fentry due to binary search in ftrace_location_range() through the whole function range.
# grep kernel_kexec /sys/kernel/tracing/available_filter_functions kernel_kexec <-- kernel_kexec+0x4/0xf8 kernel_kexec <-- kernel_kexec+0xdc/0xf8 kernel_kexec <-- kernel_kexec+0xdc/0xf8
To solve the issue, shrink the search range in first several instructions.
Fixes: fe25ad14ec35 ("livepatch: Avoid patching conflicts with kprobes") Signed-off-by: Zheng Yejian zhengyejian1@huawei.com --- arch/arm64/kernel/livepatch.c | 11 +++++++++ arch/x86/kernel/livepatch.c | 18 +++++++++++++++ kernel/livepatch/core.c | 42 +++++++++++++++++++++++++++++------ 3 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/livepatch.c b/arch/arm64/kernel/livepatch.c index 363fb8e41c49..86832f274f12 100644 --- a/arch/arm64/kernel/livepatch.c +++ b/arch/arm64/kernel/livepatch.c @@ -330,3 +330,14 @@ void arch_klp_unpatch_func(struct klp_func *func) do_patch(pc, (unsigned long)next_func->new_func); } } + +#ifdef CONFIG_LIVEPATCH_ISOLATE_KPROBE +unsigned long arch_klp_fentry_range_size(void) +{ + /* + * If first instruction is not 'bti', expect fentry exists + * at second instruction otherwise third instruction. + */ + return 3 * AARCH64_INSN_SIZE; +} +#endif /* CONFIG_LIVEPATCH_ISOLATE_KPROBE */ diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c index 99b72629637d..fdc864ec4963 100644 --- a/arch/x86/kernel/livepatch.c +++ b/arch/x86/kernel/livepatch.c @@ -386,4 +386,22 @@ void arch_klp_unpatch_func(struct klp_func *func) /* replace the text with the new text */ klp_patch_text((void *)ip, (const void *)new, JMP_E9_INSN_SIZE); } + +#ifdef CONFIG_LIVEPATCH_ISOLATE_KPROBE +unsigned long arch_klp_fentry_range_size(void) +{ + /* + * If CONFIG_X86_KERNEL_IBT enabled, there would be an 'endbr64' + * at function start, then it should be consider into the range + * size. But since CONFIG_X86_KERNEL_IBT is not introduced in this + * version, add a check here avoid this patch being port to later + * version but this case not being handled. + */ +#ifdef CONFIG_X86_KERNEL_IBT + BUILD_BUG_ON(1); +#endif + /* Expect fentry exists at first instruction. */ + return MCOUNT_INSN_SIZE; +} +#endif /* CONFIG_LIVEPATCH_ISOLATE_KPROBE */ #endif diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 3f3fa1c3c80c..92120e165bb6 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -984,6 +984,33 @@ int __weak arch_klp_init_func(struct klp_object *obj, struct klp_func *func) { return 0; } + +#ifdef CONFIG_LIVEPATCH_ISOLATE_KPROBE +unsigned long __weak arch_klp_fentry_range_size(void) +{ + return 0; +} + +static unsigned long klp_ftrace_location(unsigned long func_addr, + unsigned long func_size) +{ + unsigned long loc; + unsigned long range_size = arch_klp_fentry_range_size(); + + /* + * When some arch not need to modify codes after fentry, they are no + * need to override the weak arch_klp_fentry_range_size() which return + * 0, so just return here. + */ + if (range_size == 0 || range_size > func_size) + return 0; + loc = ftrace_location_range(func_addr, func_addr + range_size); + if (WARN_ON(loc && (loc < func_addr || loc >= func_addr + range_size))) + loc = 0; + return loc; +} +#endif /* CONFIG_LIVEPATCH_ISOLATE_KPROBE */ + #endif
static int klp_init_func(struct klp_object *obj, struct klp_func *func) @@ -1089,15 +1116,16 @@ static int klp_init_object_loaded(struct klp_patch *patch, } #ifdef CONFIG_LIVEPATCH_ISOLATE_KPROBE old_func = (unsigned long)func->old_func; - ftrace_loc = ftrace_location_range(old_func, old_func + func->old_size - 1); + ftrace_loc = klp_ftrace_location(old_func, func->old_size); if (ftrace_loc) { - if (WARN_ON(ftrace_loc < old_func || - ftrace_loc >= old_func + func->old_size - MCOUNT_INSN_SIZE)) { - pr_err("ftrace location for '%s' invalid", func->old_name); - return -EINVAL; + if (ftrace_loc >= old_func && + ftrace_loc < old_func + func->old_size - MCOUNT_INSN_SIZE) { + func->old_func = (void *)(ftrace_loc + MCOUNT_INSN_SIZE); + func->old_size -= ((unsigned long)func->old_func - old_func); + } else { + pr_warn("space not enough after ftrace location in '%s'\n", + func->old_name); } - func->old_func = (void *)(ftrace_loc + MCOUNT_INSN_SIZE); - func->old_size -= ((unsigned long)func->old_func - old_func); } #endif