From: Masami Hiramatsu <mhiramat@kernel.org> To: Alexei Starovoitov <ast@kernel.org>, Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: Daniel Borkmann <daniel@iogearbox.net>, Shubham Bansal <illusionist.neo@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Masami Hiramatsu <mhiramat@kernel.org>, bpf@vger.kernel.org, kernel-team@fb.com, Jiri Olsa <jolsa@kernel.org>, Steven Rostedt <rostedt@goodmis.org>, "Naveen N . Rao" <naveen.n.rao@linux.ibm.com>, Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>, "David S . Miller" <davem@davemloft.net>, linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>, Russell King <linux@armlinux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, linux-arm-kernel@lists.infradead.org Subject: [PATCH bpf 2/4] rethook, fprobe, kprobes: Check a failure in the rethook_hook() Date: Tue, 5 Apr 2022 18:33:59 +0900 [thread overview] Message-ID: <164915123885.982637.5653959785968470135.stgit@devnote2> (raw) In-Reply-To: <164915121498.982637.12787715964983738566.stgit@devnote2> Since there are possible to fail to hook the function return (depends on archtecutre implememtation), rethook_hook() should return the error in that case and caller must check it. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/kernel/rethook.c | 4 +++- include/linux/rethook.h | 4 ++-- kernel/kprobes.c | 8 +++++--- kernel/trace/fprobe.c | 5 ++++- kernel/trace/rethook.c | 12 ++++++++++-- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c index 8a1c0111ae79..c92b4875e3b9 100644 --- a/arch/x86/kernel/rethook.c +++ b/arch/x86/kernel/rethook.c @@ -114,7 +114,7 @@ void arch_rethook_fixup_return(struct pt_regs *regs, } NOKPROBE_SYMBOL(arch_rethook_fixup_return); -void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount) +int arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount) { unsigned long *stack = (unsigned long *)regs->sp; @@ -123,5 +123,7 @@ void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mc /* Replace the return addr with trampoline addr */ stack[0] = (unsigned long) arch_rethook_trampoline; + + return 0; } NOKPROBE_SYMBOL(arch_rethook_prepare); diff --git a/include/linux/rethook.h b/include/linux/rethook.h index c8ac1e5afcd1..07b9c6663b8e 100644 --- a/include/linux/rethook.h +++ b/include/linux/rethook.h @@ -63,12 +63,12 @@ void rethook_free(struct rethook *rh); void rethook_add_node(struct rethook *rh, struct rethook_node *node); struct rethook_node *rethook_try_get(struct rethook *rh); void rethook_recycle(struct rethook_node *node); -void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount); +int rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount); unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame, struct llist_node **cur); /* Arch dependent code must implement arch_* and trampoline code */ -void arch_rethook_prepare(struct rethook_node *node, struct pt_regs *regs, bool mcount); +int arch_rethook_prepare(struct rethook_node *node, struct pt_regs *regs, bool mcount); void arch_rethook_trampoline(void); /** diff --git a/kernel/kprobes.c b/kernel/kprobes.c index dbe57df2e199..7fd7f1195bde 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2109,10 +2109,12 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) ri = container_of(rhn, struct kretprobe_instance, node); - if (rp->entry_handler && rp->entry_handler(ri, regs)) + if (rp->entry_handler && rp->entry_handler(ri, regs)) { rethook_recycle(rhn); - else - rethook_hook(rhn, regs, kprobe_ftrace(p)); + } else if (rethook_hook(rhn, regs, kprobe_ftrace(p)) < 0) { + rethook_recycle(rhn); + rp->nmissed++; + } return 0; } diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 89d9f994ebb0..d3b13294d545 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -48,7 +48,10 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip, } fpr = container_of(rh, struct fprobe_rethook_node, node); fpr->entry_ip = ip; - rethook_hook(rh, ftrace_get_regs(fregs), true); + if (rethook_hook(rh, ftrace_get_regs(fregs), true) < 0) { + rethook_recycle(rh); + fp->nmissed++; + } } out: diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c index b56833700d23..e7db83438e45 100644 --- a/kernel/trace/rethook.c +++ b/kernel/trace/rethook.c @@ -174,11 +174,19 @@ NOKPROBE_SYMBOL(rethook_try_get); * from ftrace (mcount) callback, @mcount must be set true. If this is called * from the real function entry (e.g. kprobes) @mcount must be set false. * This is because the way to hook the function return depends on the context. + * This returns 0 if succeeded to hook the function return, or -errno if + * failed. */ -void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount) +int rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount) { - arch_rethook_prepare(node, regs, mcount); + int ret; + + ret = arch_rethook_prepare(node, regs, mcount); + if (ret < 0) + return ret; + __llist_add(&node->llist, ¤t->rethooks); + return 0; } NOKPROBE_SYMBOL(rethook_hook); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org> To: Alexei Starovoitov <ast@kernel.org>, Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: Daniel Borkmann <daniel@iogearbox.net>, Shubham Bansal <illusionist.neo@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, Masami Hiramatsu <mhiramat@kernel.org>, bpf@vger.kernel.org, kernel-team@fb.com, Jiri Olsa <jolsa@kernel.org>, Steven Rostedt <rostedt@goodmis.org>, "Naveen N . Rao" <naveen.n.rao@linux.ibm.com>, Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>, "David S . Miller" <davem@davemloft.net>, linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>, Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>, Russell King <linux@armlinux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, linux-arm-kernel@lists.infradead.org Subject: [PATCH bpf 2/4] rethook,fprobe,kprobes: Check a failure in the rethook_hook() Date: Tue, 5 Apr 2022 18:33:59 +0900 [thread overview] Message-ID: <164915123885.982637.5653959785968470135.stgit@devnote2> (raw) In-Reply-To: <164915121498.982637.12787715964983738566.stgit@devnote2> Since there are possible to fail to hook the function return (depends on archtecutre implememtation), rethook_hook() should return the error in that case and caller must check it. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/kernel/rethook.c | 4 +++- include/linux/rethook.h | 4 ++-- kernel/kprobes.c | 8 +++++--- kernel/trace/fprobe.c | 5 ++++- kernel/trace/rethook.c | 12 ++++++++++-- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c index 8a1c0111ae79..c92b4875e3b9 100644 --- a/arch/x86/kernel/rethook.c +++ b/arch/x86/kernel/rethook.c @@ -114,7 +114,7 @@ void arch_rethook_fixup_return(struct pt_regs *regs, } NOKPROBE_SYMBOL(arch_rethook_fixup_return); -void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount) +int arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount) { unsigned long *stack = (unsigned long *)regs->sp; @@ -123,5 +123,7 @@ void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mc /* Replace the return addr with trampoline addr */ stack[0] = (unsigned long) arch_rethook_trampoline; + + return 0; } NOKPROBE_SYMBOL(arch_rethook_prepare); diff --git a/include/linux/rethook.h b/include/linux/rethook.h index c8ac1e5afcd1..07b9c6663b8e 100644 --- a/include/linux/rethook.h +++ b/include/linux/rethook.h @@ -63,12 +63,12 @@ void rethook_free(struct rethook *rh); void rethook_add_node(struct rethook *rh, struct rethook_node *node); struct rethook_node *rethook_try_get(struct rethook *rh); void rethook_recycle(struct rethook_node *node); -void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount); +int rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount); unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame, struct llist_node **cur); /* Arch dependent code must implement arch_* and trampoline code */ -void arch_rethook_prepare(struct rethook_node *node, struct pt_regs *regs, bool mcount); +int arch_rethook_prepare(struct rethook_node *node, struct pt_regs *regs, bool mcount); void arch_rethook_trampoline(void); /** diff --git a/kernel/kprobes.c b/kernel/kprobes.c index dbe57df2e199..7fd7f1195bde 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2109,10 +2109,12 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs) ri = container_of(rhn, struct kretprobe_instance, node); - if (rp->entry_handler && rp->entry_handler(ri, regs)) + if (rp->entry_handler && rp->entry_handler(ri, regs)) { rethook_recycle(rhn); - else - rethook_hook(rhn, regs, kprobe_ftrace(p)); + } else if (rethook_hook(rhn, regs, kprobe_ftrace(p)) < 0) { + rethook_recycle(rhn); + rp->nmissed++; + } return 0; } diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 89d9f994ebb0..d3b13294d545 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -48,7 +48,10 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip, } fpr = container_of(rh, struct fprobe_rethook_node, node); fpr->entry_ip = ip; - rethook_hook(rh, ftrace_get_regs(fregs), true); + if (rethook_hook(rh, ftrace_get_regs(fregs), true) < 0) { + rethook_recycle(rh); + fp->nmissed++; + } } out: diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c index b56833700d23..e7db83438e45 100644 --- a/kernel/trace/rethook.c +++ b/kernel/trace/rethook.c @@ -174,11 +174,19 @@ NOKPROBE_SYMBOL(rethook_try_get); * from ftrace (mcount) callback, @mcount must be set true. If this is called * from the real function entry (e.g. kprobes) @mcount must be set false. * This is because the way to hook the function return depends on the context. + * This returns 0 if succeeded to hook the function return, or -errno if + * failed. */ -void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount) +int rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount) { - arch_rethook_prepare(node, regs, mcount); + int ret; + + ret = arch_rethook_prepare(node, regs, mcount); + if (ret < 0) + return ret; + __llist_add(&node->llist, ¤t->rethooks); + return 0; } NOKPROBE_SYMBOL(rethook_hook);
next prev parent reply other threads:[~2022-04-05 9:36 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-05 9:33 [PATCH bpf 0/4] kprobes: rethook, ARM, arm64: Replace kretprobe trampoline with rethook Masami Hiramatsu 2022-04-05 9:33 ` [PATCH bpf 0/4] kprobes: rethook,ARM,arm64: " Masami Hiramatsu 2022-04-05 9:33 ` [PATCH bpf 1/4] ARM: unwind: Initialize the lr_addr field of unwind_ctrl_block Masami Hiramatsu 2022-04-05 9:33 ` Masami Hiramatsu 2022-04-06 18:53 ` kernel test robot 2022-04-06 18:53 ` kernel test robot 2022-04-05 9:33 ` Masami Hiramatsu [this message] 2022-04-05 9:33 ` [PATCH bpf 2/4] rethook,fprobe,kprobes: Check a failure in the rethook_hook() Masami Hiramatsu 2022-04-05 9:34 ` [PATCH bpf 3/4] ARM: rethook: Replace kretprobe trampoline with rethook Masami Hiramatsu 2022-04-05 9:34 ` Masami Hiramatsu 2022-04-05 9:34 ` [PATCH bpf 4/4] arm64: " Masami Hiramatsu 2022-04-05 9:34 ` Masami Hiramatsu 2022-04-05 17:40 ` kernel test robot 2022-04-05 17:40 ` kernel test robot 2022-04-05 21:25 ` kernel test robot 2022-04-05 21:25 ` kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=164915123885.982637.5653959785968470135.stgit@devnote2 \ --to=mhiramat@kernel.org \ --cc=alexei.starovoitov@gmail.com \ --cc=andrii@kernel.org \ --cc=anil.s.keshavamurthy@intel.com \ --cc=ardb@kernel.org \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=catalin.marinas@arm.com \ --cc=daniel@iogearbox.net \ --cc=davem@davemloft.net \ --cc=illusionist.neo@gmail.com \ --cc=jolsa@kernel.org \ --cc=kernel-team@fb.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mark.rutland@arm.com \ --cc=naveen.n.rao@linux.ibm.com \ --cc=rostedt@goodmis.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.