NACK: [SRU][jammy][PATCH v2 3/5] kprobes: Add kretprobe_find_ret_addr() for searching return address
Stefan Bader
stefan.bader at canonical.com
Thu Apr 6 13:42:09 UTC 2023
On 31.03.23 16:00, John Cabaj wrote:
> From: Masami Hiramatsu <mhiramat at 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 at kernel.org>
> Tested-by: Andrii Nakryiko <andrii at kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt at goodmis.org>
> (backported from commit 03bac0df2886882c43e6d0bfff9dee84a184fc7e)
> [John Cabaj: cleaning-up kernel panic that was relocated to conditional]
> Signed-off-by: John Cabaj <john.cabaj at 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);
> }
--
- Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230406/b4203cad/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230406/b4203cad/attachment-0001.sig>
More information about the kernel-team
mailing list