| Message ID | 20230331140041.2112510-4-john.cabaj@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | ftrace graph return address recovery support for s390x Livepatch | expand |
On 31.03.23 16:00, John Cabaj wrote: > From: Masami Hiramatsu <mhiramat@kernel.org> > > BugLink: https://bugs.launchpad.net/bugs/2013603 (Kernel livepatch ftrace graph fix) > > Introduce kretprobe_find_ret_addr() and is_kretprobe_trampoline(). > These APIs will be used by the ORC stack unwinder and ftrace, so that > they can check whether the given address points kretprobe trampoline > code and query the correct return address in that case. > > Link: https://lkml.kernel.org/r/163163046461.489837.1044778356430293962.stgit@devnote2 > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > Tested-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > (backported from commit 03bac0df2886882c43e6d0bfff9dee84a184fc7e) > [John Cabaj: cleaning-up kernel panic that was relocated to conditional] > Signed-off-by: John Cabaj <john.cabaj@canonical.com> > --- > include/linux/kprobes.h | 22 ++++++++ > kernel/kprobes.c | 109 ++++++++++++++++++++++++++++++---------- > 2 files changed, 105 insertions(+), 26 deletions(-) > > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index f6b0aef35f66..9e24d8b79100 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -511,6 +511,28 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) > } > #endif > > +#ifdef CONFIG_KRETPROBES > +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) > +{ > + return (void *)addr == kretprobe_trampoline_addr(); > +} > + > +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, > + struct llist_node **cur); > +#else > +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) > +{ > + return false; > +} > + > +static nokprobe_inline > +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, > + struct llist_node **cur) > +{ > + return 0; > +} > +#endif > + > /* Returns true if kprobes handled the fault */ > static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, > unsigned int trap) The set fails to apply to current jammy:linux/master-next: Applying: kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() Applying: kprobes: treewide: Make it harder to refer kretprobe_trampoline directly Applying: kprobes: Add kretprobe_find_ret_addr() for searching return address error: patch failed: kernel/kprobes.c:1866 error: kernel/kprobes.c: patch does not apply Patch failed at 0003 kprobes: Add kretprobe_find_ret_addr() for searching return address checking file include/linux/kprobes.h checking file kernel/kprobes.c Hunk #1 FAILED at 1866. Hunk #2 succeeded at 1913 (offset -2 lines). 1 out of 2 hunks FAILED Please submit again (v3) once this is fixed. -Stefan > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 7da4a5048eb7..8e06a8d6516c 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1866,45 +1866,87 @@ unsigned long __weak arch_deref_entry_point(void *entry) > > #ifdef CONFIG_KRETPROBES > > -unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, > - void *frame_pointer) > +/* This assumes the 'tsk' is the current task or the is not running. */ > +static kprobe_opcode_t *__kretprobe_find_ret_addr(struct task_struct *tsk, > + struct llist_node **cur) > { > - kprobe_opcode_t *correct_ret_addr = NULL; > struct kretprobe_instance *ri = NULL; > - struct llist_node *first, *node; > - struct kretprobe *rp; > + struct llist_node *node = *cur; > + > + if (!node) > + node = tsk->kretprobe_instances.first; > + else > + node = node->next; > > - /* Find all nodes for this frame. */ > - first = node = current->kretprobe_instances.first; > while (node) { > ri = container_of(node, struct kretprobe_instance, llist); > - > - BUG_ON(ri->fp != frame_pointer); > - > if (ri->ret_addr != kretprobe_trampoline_addr()) { > - correct_ret_addr = ri->ret_addr; > - /* > - * This is the real return address. Any other > - * instances associated with this task are for > - * other calls deeper on the call stack > - */ > - goto found; > + *cur = node; > + return ri->ret_addr; > } > - > node = node->next; > } > - pr_err("Oops! Kretprobe fails to find correct return address.\n"); > - BUG_ON(1); > + return NULL; > +} > +NOKPROBE_SYMBOL(__kretprobe_find_ret_addr); > > -found: > - /* Unlink all nodes for this frame. */ > - current->kretprobe_instances.first = node->next; > - node->next = NULL; > +/** > + * kretprobe_find_ret_addr -- Find correct return address modified by kretprobe > + * @tsk: Target task > + * @fp: A frame pointer > + * @cur: a storage of the loop cursor llist_node pointer for next call > + * > + * Find the correct return address modified by a kretprobe on @tsk in unsigned > + * long type. If it finds the return address, this returns that address value, > + * or this returns 0. > + * The @tsk must be 'current' or a task which is not running. @fp is a hint > + * to get the currect return address - which is compared with the > + * kretprobe_instance::fp field. The @cur is a loop cursor for searching the > + * kretprobe return addresses on the @tsk. The '*@cur' should be NULL at the > + * first call, but '@cur' itself must NOT NULL. > + */ > +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, > + struct llist_node **cur) > +{ > + struct kretprobe_instance *ri = NULL; > + kprobe_opcode_t *ret; > + > + if (WARN_ON_ONCE(!cur)) > + return 0; > + > + do { > + ret = __kretprobe_find_ret_addr(tsk, cur); > + if (!ret) > + break; > + ri = container_of(*cur, struct kretprobe_instance, llist); > + } while (ri->fp != fp); > > - /* Run them.. */ > + return (unsigned long)ret; > +} > +NOKPROBE_SYMBOL(kretprobe_find_ret_addr); > + > +unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, > + void *frame_pointer) > +{ > + kprobe_opcode_t *correct_ret_addr = NULL; > + struct kretprobe_instance *ri = NULL; > + struct llist_node *first, *node = NULL; > + struct kretprobe *rp; > + > + /* Find correct address and all nodes for this frame. */ > + correct_ret_addr = __kretprobe_find_ret_addr(current, &node); > + if (!correct_ret_addr) { > + pr_err("kretprobe: Return address not found, not execute handler. Maybe there is a bug in the kernel.\n"); > + BUG_ON(1); > + } > + > + /* Run the user handler of the nodes. */ > + first = current->kretprobe_instances.first; > while (first) { > ri = container_of(first, struct kretprobe_instance, llist); > - first = first->next; > + > + if (WARN_ON_ONCE(ri->fp != frame_pointer)) > + break; > > rp = get_kretprobe(ri); > if (rp && rp->handler) { > @@ -1915,6 +1957,21 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, > rp->handler(ri, regs); > __this_cpu_write(current_kprobe, prev); > } > + if (first == node) > + break; > + > + first = first->next; > + } > + > + /* Unlink all nodes for this frame. */ > + first = current->kretprobe_instances.first; > + current->kretprobe_instances.first = node->next; > + node->next = NULL; > + > + /* Recycle free instances. */ > + while (first) { > + ri = container_of(first, struct kretprobe_instance, llist); > + first = first->next; > > recycle_rp_inst(ri); > }
On 4/6/23 8:42 AM, Stefan Bader wrote: > On 31.03.23 16:00, John Cabaj wrote: >> From: Masami Hiramatsu <mhiramat@kernel.org> >> >> BugLink: https://bugs.launchpad.net/bugs/2013603 (Kernel livepatch ftrace graph fix) >> >> Introduce kretprobe_find_ret_addr() and is_kretprobe_trampoline(). >> These APIs will be used by the ORC stack unwinder and ftrace, so that >> they can check whether the given address points kretprobe trampoline >> code and query the correct return address in that case. >> >> Link: https://lkml.kernel.org/r/163163046461.489837.1044778356430293962.stgit@devnote2 >> >> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> >> Tested-by: Andrii Nakryiko <andrii@kernel.org> >> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> >> (backported from commit 03bac0df2886882c43e6d0bfff9dee84a184fc7e) >> [John Cabaj: cleaning-up kernel panic that was relocated to conditional] >> Signed-off-by: John Cabaj <john.cabaj@canonical.com> >> --- >> include/linux/kprobes.h | 22 ++++++++ >> kernel/kprobes.c | 109 ++++++++++++++++++++++++++++++---------- >> 2 files changed, 105 insertions(+), 26 deletions(-) >> >> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h >> index f6b0aef35f66..9e24d8b79100 100644 >> --- a/include/linux/kprobes.h >> +++ b/include/linux/kprobes.h >> @@ -511,6 +511,28 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) >> } >> #endif >> +#ifdef CONFIG_KRETPROBES >> +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) >> +{ >> + return (void *)addr == kretprobe_trampoline_addr(); >> +} >> + >> +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, >> + struct llist_node **cur); >> +#else >> +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) >> +{ >> + return false; >> +} >> + >> +static nokprobe_inline >> +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, >> + struct llist_node **cur) >> +{ >> + return 0; >> +} >> +#endif >> + >> /* Returns true if kprobes handled the fault */ >> static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, >> unsigned int trap) > > The set fails to apply to current jammy:linux/master-next: > > Applying: kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() > Applying: kprobes: treewide: Make it harder to refer kretprobe_trampoline directly > Applying: kprobes: Add kretprobe_find_ret_addr() for searching return address > error: patch failed: kernel/kprobes.c:1866 > error: kernel/kprobes.c: patch does not apply > Patch failed at 0003 kprobes: Add kretprobe_find_ret_addr() for searching return address > > checking file include/linux/kprobes.h > checking file kernel/kprobes.c > Hunk #1 FAILED at 1866. > Hunk #2 succeeded at 1913 (offset -2 lines). > 1 out of 2 hunks FAILED This likely collided with changes being applied since this patch was originally submitted, possibly c3694073333c ("kprobes: treewide: Cleanup the error messages for kprobes") Resubmitted and checked with a re-application - hopefully it doesn't conflict with more kprobes changes before next application. Thanks, John > > Please submit again (v3) once this is fixed. > > -Stefan > >> diff --git a/kernel/kprobes.c b/kernel/kprobes.c >> index 7da4a5048eb7..8e06a8d6516c 100644 >> --- a/kernel/kprobes.c >> +++ b/kernel/kprobes.c >> @@ -1866,45 +1866,87 @@ unsigned long __weak arch_deref_entry_point(void *entry) >> #ifdef CONFIG_KRETPROBES >> -unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, >> - void *frame_pointer) >> +/* This assumes the 'tsk' is the current task or the is not running. */ >> +static kprobe_opcode_t *__kretprobe_find_ret_addr(struct task_struct *tsk, >> + struct llist_node **cur) >> { >> - kprobe_opcode_t *correct_ret_addr = NULL; >> struct kretprobe_instance *ri = NULL; >> - struct llist_node *first, *node; >> - struct kretprobe *rp; >> + struct llist_node *node = *cur; >> + >> + if (!node) >> + node = tsk->kretprobe_instances.first; >> + else >> + node = node->next; >> - /* Find all nodes for this frame. */ >> - first = node = current->kretprobe_instances.first; >> while (node) { >> ri = container_of(node, struct kretprobe_instance, llist); >> - >> - BUG_ON(ri->fp != frame_pointer); >> - >> if (ri->ret_addr != kretprobe_trampoline_addr()) { >> - correct_ret_addr = ri->ret_addr; >> - /* >> - * This is the real return address. Any other >> - * instances associated with this task are for >> - * other calls deeper on the call stack >> - */ >> - goto found; >> + *cur = node; >> + return ri->ret_addr; >> } >> - >> node = node->next; >> } >> - pr_err("Oops! Kretprobe fails to find correct return address.\n"); >> - BUG_ON(1); >> + return NULL; >> +} >> +NOKPROBE_SYMBOL(__kretprobe_find_ret_addr); >> -found: >> - /* Unlink all nodes for this frame. */ >> - current->kretprobe_instances.first = node->next; >> - node->next = NULL; >> +/** >> + * kretprobe_find_ret_addr -- Find correct return address modified by kretprobe >> + * @tsk: Target task >> + * @fp: A frame pointer >> + * @cur: a storage of the loop cursor llist_node pointer for next call >> + * >> + * Find the correct return address modified by a kretprobe on @tsk in unsigned >> + * long type. If it finds the return address, this returns that address value, >> + * or this returns 0. >> + * The @tsk must be 'current' or a task which is not running. @fp is a hint >> + * to get the currect return address - which is compared with the >> + * kretprobe_instance::fp field. The @cur is a loop cursor for searching the >> + * kretprobe return addresses on the @tsk. The '*@cur' should be NULL at the >> + * first call, but '@cur' itself must NOT NULL. >> + */ >> +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, >> + struct llist_node **cur) >> +{ >> + struct kretprobe_instance *ri = NULL; >> + kprobe_opcode_t *ret; >> + >> + if (WARN_ON_ONCE(!cur)) >> + return 0; >> + >> + do { >> + ret = __kretprobe_find_ret_addr(tsk, cur); >> + if (!ret) >> + break; >> + ri = container_of(*cur, struct kretprobe_instance, llist); >> + } while (ri->fp != fp); >> - /* Run them.. */ >> + return (unsigned long)ret; >> +} >> +NOKPROBE_SYMBOL(kretprobe_find_ret_addr); >> + >> +unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, >> + void *frame_pointer) >> +{ >> + kprobe_opcode_t *correct_ret_addr = NULL; >> + struct kretprobe_instance *ri = NULL; >> + struct llist_node *first, *node = NULL; >> + struct kretprobe *rp; >> + >> + /* Find correct address and all nodes for this frame. */ >> + correct_ret_addr = __kretprobe_find_ret_addr(current, &node); >> + if (!correct_ret_addr) { >> + pr_err("kretprobe: Return address not found, not execute handler. Maybe there is a bug in the kernel.\n"); >> + BUG_ON(1); >> + } >> + >> + /* Run the user handler of the nodes. */ >> + first = current->kretprobe_instances.first; >> while (first) { >> ri = container_of(first, struct kretprobe_instance, llist); >> - first = first->next; >> + >> + if (WARN_ON_ONCE(ri->fp != frame_pointer)) >> + break; >> rp = get_kretprobe(ri); >> if (rp && rp->handler) { >> @@ -1915,6 +1957,21 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, >> rp->handler(ri, regs); >> __this_cpu_write(current_kprobe, prev); >> } >> + if (first == node) >> + break; >> + >> + first = first->next; >> + } >> + >> + /* Unlink all nodes for this frame. */ >> + first = current->kretprobe_instances.first; >> + current->kretprobe_instances.first = node->next; >> + node->next = NULL; >> + >> + /* Recycle free instances. */ >> + while (first) { >> + ri = container_of(first, struct kretprobe_instance, llist); >> + first = first->next; >> recycle_rp_inst(ri); >> } >
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index f6b0aef35f66..9e24d8b79100 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -511,6 +511,28 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) } #endif +#ifdef CONFIG_KRETPROBES +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) +{ + return (void *)addr == kretprobe_trampoline_addr(); +} + +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, + struct llist_node **cur); +#else +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) +{ + return false; +} + +static nokprobe_inline +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, + struct llist_node **cur) +{ + return 0; +} +#endif + /* Returns true if kprobes handled the fault */ static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, unsigned int trap) diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 7da4a5048eb7..8e06a8d6516c 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1866,45 +1866,87 @@ unsigned long __weak arch_deref_entry_point(void *entry) #ifdef CONFIG_KRETPROBES -unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, - void *frame_pointer) +/* This assumes the 'tsk' is the current task or the is not running. */ +static kprobe_opcode_t *__kretprobe_find_ret_addr(struct task_struct *tsk, + struct llist_node **cur) { - kprobe_opcode_t *correct_ret_addr = NULL; struct kretprobe_instance *ri = NULL; - struct llist_node *first, *node; - struct kretprobe *rp; + struct llist_node *node = *cur; + + if (!node) + node = tsk->kretprobe_instances.first; + else + node = node->next; - /* Find all nodes for this frame. */ - first = node = current->kretprobe_instances.first; while (node) { ri = container_of(node, struct kretprobe_instance, llist); - - BUG_ON(ri->fp != frame_pointer); - if (ri->ret_addr != kretprobe_trampoline_addr()) { - correct_ret_addr = ri->ret_addr; - /* - * This is the real return address. Any other - * instances associated with this task are for - * other calls deeper on the call stack - */ - goto found; + *cur = node; + return ri->ret_addr; } - node = node->next; } - pr_err("Oops! Kretprobe fails to find correct return address.\n"); - BUG_ON(1); + return NULL; +} +NOKPROBE_SYMBOL(__kretprobe_find_ret_addr); -found: - /* Unlink all nodes for this frame. */ - current->kretprobe_instances.first = node->next; - node->next = NULL; +/** + * kretprobe_find_ret_addr -- Find correct return address modified by kretprobe + * @tsk: Target task + * @fp: A frame pointer + * @cur: a storage of the loop cursor llist_node pointer for next call + * + * Find the correct return address modified by a kretprobe on @tsk in unsigned + * long type. If it finds the return address, this returns that address value, + * or this returns 0. + * The @tsk must be 'current' or a task which is not running. @fp is a hint + * to get the currect return address - which is compared with the + * kretprobe_instance::fp field. The @cur is a loop cursor for searching the + * kretprobe return addresses on the @tsk. The '*@cur' should be NULL at the + * first call, but '@cur' itself must NOT NULL. + */ +unsigned long kretprobe_find_ret_addr(struct task_struct *tsk, void *fp, + struct llist_node **cur) +{ + struct kretprobe_instance *ri = NULL; + kprobe_opcode_t *ret; + + if (WARN_ON_ONCE(!cur)) + return 0; + + do { + ret = __kretprobe_find_ret_addr(tsk, cur); + if (!ret) + break; + ri = container_of(*cur, struct kretprobe_instance, llist); + } while (ri->fp != fp); - /* Run them.. */ + return (unsigned long)ret; +} +NOKPROBE_SYMBOL(kretprobe_find_ret_addr); + +unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, + void *frame_pointer) +{ + kprobe_opcode_t *correct_ret_addr = NULL; + struct kretprobe_instance *ri = NULL; + struct llist_node *first, *node = NULL; + struct kretprobe *rp; + + /* Find correct address and all nodes for this frame. */ + correct_ret_addr = __kretprobe_find_ret_addr(current, &node); + if (!correct_ret_addr) { + pr_err("kretprobe: Return address not found, not execute handler. Maybe there is a bug in the kernel.\n"); + BUG_ON(1); + } + + /* Run the user handler of the nodes. */ + first = current->kretprobe_instances.first; while (first) { ri = container_of(first, struct kretprobe_instance, llist); - first = first->next; + + if (WARN_ON_ONCE(ri->fp != frame_pointer)) + break; rp = get_kretprobe(ri); if (rp && rp->handler) { @@ -1915,6 +1957,21 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, rp->handler(ri, regs); __this_cpu_write(current_kprobe, prev); } + if (first == node) + break; + + first = first->next; + } + + /* Unlink all nodes for this frame. */ + first = current->kretprobe_instances.first; + current->kretprobe_instances.first = node->next; + node->next = NULL; + + /* Recycle free instances. */ + while (first) { + ri = container_of(first, struct kretprobe_instance, llist); + first = first->next; recycle_rp_inst(ri); }