hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7BMC9 CVE: NA -------------------------------- Suppose task 'T' is running in function 'B', and user is doing livepatch for function 'A', and function 'A' is calling 'B'. Then task 'T' will be interrupted by stop_machine, and suppose that 'B' has not saved the link register (which point to address in 'A') into stack memory, and data in link register position may be invalid, then 'A' would be missed on stack checking. To find a solution, we list following cases (suppose calling chain: P -> A -> B, A runs `bl B`, 'nip' means instruction pointer found in interrupt frame, 'lr' means link register found in interrupt frame, 'frame->pc' means link register saved in first stack frame): 1. 'nip' is in 'A', `bl B` is not executed, then: 'lr' is in 'P', 'frame->pc' should be ignored; 2. 'nip' is in some ppc64_stub or ppc32_plt which also not save link register, then: 'lr' is in 'A', 'frame->pc' should be ignored; 3. 'nip' is in 'B', but 'B' has not saved link register, then: 'lr' is in 'A', 'frame->pc' should be ignored; 4. 'nip' is in 'B', 'B' has saved link register, then: 'lr' is in 'A', 'frame->pc' is also in 'A'; 5. 'nip' is in 'A', 'B' has returned but its stack pointer is not moved, then 'lr' is in 'A', 'frame->pc' is also in 'A'; 6. 'nip' is in 'A', 'B' has returned and its stack pointer is moved, then 'lr' is in 'A', 'frame->pc' is in 'P'. As a conclusion, we need to: 1. check 'nip' or 'lr' or both if they are not in same function; 2. ignore 'frame->pc' if 'nip' and 'lr' are not in same function. Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com> --- arch/powerpc/include/asm/livepatch.h | 11 ++++++++++ arch/powerpc/kernel/livepatch.c | 28 +++++++++++++++++++++++-- arch/powerpc/kernel/livepatch_32.c | 31 +++++++++++++++++++--------- arch/powerpc/kernel/livepatch_64.c | 31 +++++++++++++++++++--------- 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h index ae674ea59ab3..285602e637f1 100644 --- a/arch/powerpc/include/asm/livepatch.h +++ b/arch/powerpc/include/asm/livepatch.h @@ -121,9 +121,20 @@ struct arch_klp_data { #define KLP_MAX_REPLACE_SIZE sizeof_field(struct arch_klp_data, old_insns) struct stackframe { + /* stack frame to be unwinded */ unsigned long sp; + /* link register saved in last stack frame */ unsigned long pc; + /* instruction register saved in pt_regs */ unsigned long nip; + /* link register saved in pt_regs */ + unsigned long link; + /* stack frame pointer (r1 register) saved in pt_regs */ + unsigned long sfp; + /* check if nip and link are in same function */ + unsigned int nip_link_in_same_func; + /* check if it is top frame before interrupt */ + unsigned int is_top_frame; }; #ifdef PPC64_ELF_ABI_v1 diff --git a/arch/powerpc/kernel/livepatch.c b/arch/powerpc/kernel/livepatch.c index d568e8c8b16b..5ba38c2c7c5c 100644 --- a/arch/powerpc/kernel/livepatch.c +++ b/arch/powerpc/kernel/livepatch.c @@ -23,6 +23,7 @@ #include <linux/module.h> #include <linux/ftrace.h> #include <linux/livepatch.h> +#include <linux/kallsyms.h> #include <asm/probes.h> #include <asm/livepatch.h> #include <asm/code-patching.h> @@ -67,6 +68,22 @@ int klp_brk_handler(struct pt_regs *regs) return 1; } +static int check_addr_in_same_func(unsigned long addr1, unsigned long addr2) +{ + unsigned long size = 0; + unsigned long offset = 0; + unsigned long start; + + if (addr1 == 0 || addr2 == 0) + return 0; + if (addr1 == addr2) + return 1; + if (!kallsyms_lookup_size_offset(addr1, &size, &offset)) + return 0; + start = addr1 - offset; + return (addr2 >= start) && (addr2 - start < size); +} + int klp_unwind_frame(struct task_struct *tsk, struct stackframe *frame) { unsigned long *stack; @@ -79,7 +96,10 @@ int klp_unwind_frame(struct task_struct *tsk, struct stackframe *frame) if (frame->nip != 0) frame->nip = 0; + if (frame->link != 0) + frame->link = 0; + frame->is_top_frame = (frame->sfp == frame->sp); stack = (unsigned long *)frame->sp; /* @@ -94,10 +114,14 @@ int klp_unwind_frame(struct task_struct *tsk, struct stackframe *frame) struct pt_regs *regs = (struct pt_regs *) (frame->sp + STACK_FRAME_OVERHEAD); frame->nip = regs->nip; - pr_debug("--- interrupt: task = %d/%s, trap %lx at NIP=x%lx/%pS, LR=0x%lx/%pS\n", + frame->link = regs->link; + frame->sfp = regs->gpr[PT_R1]; + frame->nip_link_in_same_func = check_addr_in_same_func(frame->nip, frame->link); + pr_debug("--- interrupt: task = %d/%s, trap %lx at NIP=0x%lx/%pS, LR=0x%lx/%pS, SFP=0x%lx, nip_link_in_same_func=%u\n", tsk->pid, tsk->comm, regs->trap, regs->nip, (void *)regs->nip, - regs->link, (void *)regs->link); + regs->link, (void *)regs->link, + frame->sfp, frame->nip_link_in_same_func); } frame->sp = stack[0]; diff --git a/arch/powerpc/kernel/livepatch_32.c b/arch/powerpc/kernel/livepatch_32.c index 3fe4f3c5790b..134b08e12e74 100644 --- a/arch/powerpc/kernel/livepatch_32.c +++ b/arch/powerpc/kernel/livepatch_32.c @@ -272,13 +272,21 @@ static int klp_check_jump_func(struct stackframe *frame, void *data) struct walk_stackframe_args *args = data; struct klp_func_list *check_funcs = args->check_funcs; - /* check the PC first */ - if (!check_func_list(check_funcs, &args->ret, frame->pc)) - return args->ret; - /* check NIP when the exception stack switching */ if (frame->nip && !check_func_list(check_funcs, &args->ret, frame->nip)) return args->ret; + if (frame->link && !frame->nip_link_in_same_func && + !check_func_list(check_funcs, &args->ret, frame->link)) + return args->ret; + /* + * There are two cases that frame->pc is reliable: + * 1. frame->pc is not in top frame before interrupt; + * 2. nip and link are in same function; + */ + if (!frame->is_top_frame || frame->nip_link_in_same_func) { + if (!check_func_list(check_funcs, &args->ret, frame->pc)) + return args->ret; + } return 0; } @@ -298,10 +306,11 @@ static int do_check_calltrace(struct walk_stackframe_args *args, int (*fn)(struct stackframe *, void *)) { struct task_struct *g, *t; - struct stackframe frame; unsigned long *stack; for_each_process_thread(g, t) { + struct stackframe frame = { 0 }; + if (t == current) { /* * Handle the current carefully on each CPUs, we shouldn't @@ -329,7 +338,6 @@ static int do_check_calltrace(struct walk_stackframe_args *args, frame.sp = (unsigned long)stack; frame.pc = stack[STACK_FRAME_LR_SAVE]; - frame.nip = 0; klp_walk_stackframe(&frame, fn, t, args); if (args->ret) { pr_info("PID: %d Comm: %.20s\n", t->pid, t->comm); @@ -368,13 +376,16 @@ static int check_module_calltrace(struct stackframe *frame, void *data) { struct walk_stackframe_args *args = data; - /* check the PC first */ - if (within_module_core(frame->pc, args->mod)) - goto err_out; - /* check NIP when the exception stack switching */ if (frame->nip && within_module_core(frame->nip, args->mod)) goto err_out; + if (frame->link && !frame->nip_link_in_same_func && + within_module_core(frame->link, args->mod)) + goto err_out; + if (!frame->is_top_frame || frame->nip_link_in_same_func) { + if (within_module_core(frame->pc, args->mod)) + goto err_out; + } return 0; diff --git a/arch/powerpc/kernel/livepatch_64.c b/arch/powerpc/kernel/livepatch_64.c index a2ec7c8c1bad..b33839b5916a 100644 --- a/arch/powerpc/kernel/livepatch_64.c +++ b/arch/powerpc/kernel/livepatch_64.c @@ -273,13 +273,21 @@ static int klp_check_jump_func(struct stackframe *frame, void *data) struct walk_stackframe_args *args = data; struct klp_func_list *check_funcs = args->check_funcs; - /* check the PC first */ - if (!check_func_list(check_funcs, &args->ret, frame->pc)) - return args->ret; - /* check NIP when the exception stack switching */ if (frame->nip && !check_func_list(check_funcs, &args->ret, frame->nip)) return args->ret; + if (frame->link && !frame->nip_link_in_same_func && + !check_func_list(check_funcs, &args->ret, frame->link)) + return args->ret; + /* + * There are two cases that frame->pc is reliable: + * 1. frame->pc is not in top frame before interrupt; + * 2. nip and link are in same function; + */ + if (!frame->is_top_frame || frame->nip_link_in_same_func) { + if (!check_func_list(check_funcs, &args->ret, frame->pc)) + return args->ret; + } return 0; } @@ -299,10 +307,11 @@ static int do_check_calltrace(struct walk_stackframe_args *args, int (*fn)(struct stackframe *, void *)) { struct task_struct *g, *t; - struct stackframe frame; unsigned long *stack; for_each_process_thread(g, t) { + struct stackframe frame = { 0 }; + if (t == current) { /* * Handle the current carefully on each CPUs, @@ -332,7 +341,6 @@ static int do_check_calltrace(struct walk_stackframe_args *args, frame.sp = (unsigned long)stack; frame.pc = stack[STACK_FRAME_LR_SAVE]; - frame.nip = 0; klp_walk_stackframe(&frame, fn, t, args); if (args->ret) { pr_debug("%s FAILED when %s\n", __func__, @@ -373,13 +381,16 @@ static int check_module_calltrace(struct stackframe *frame, void *data) { struct walk_stackframe_args *args = data; - /* check the PC first */ - if (within_module_core(frame->pc, args->mod)) - goto err_out; - /* check NIP when the exception stack switching */ if (frame->nip && within_module_core(frame->nip, args->mod)) goto err_out; + if (frame->link && !frame->nip_link_in_same_func && + within_module_core(frame->link, args->mod)) + goto err_out; + if (!frame->is_top_frame || frame->nip_link_in_same_func) { + if (within_module_core(frame->pc, args->mod)) + goto err_out; + } return 0; -- 2.25.1