From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CA523C77B7A for ; Wed, 17 May 2023 14:28:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: Message-Id:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PR87QhvqWIecdJt41+ktZ/M+kLfVyWpZni15gJR8q3g=; b=A6KC43fNXhiI9P zDVOjw1krOfqOqFat1SRbF1tN35CAt1k+sBD7GIjfyj1A52rtuu9omBZombspQug4wwaUowHh1pVm oAhSze3yVGqNKKXCJklaaB7pf+BoxKP5AAiEt1N5N1a9PWpz+rYzchgSB9i5FgSOdZRGng/PrM1WM nzlNTlTNxBsjTV5VH9+5YIFf9hNEvHV2zHHGffj9zkHj6GyWmUQbq5/Ju8vybsaMYAtt3l/GNSB51 s/CL/ij2bpdmYFyUx+vclwt8HuOE5gNt2u0W2cCpHm81yCrseVmay81k71KrPpbBkmyMw3OB1uuBw kAX1CoO+IpkasCrvOXSA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pzI8J-00A85t-1q; Wed, 17 May 2023 14:28:03 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pzI8F-00A83p-18 for linux-riscv@lists.infradead.org; Wed, 17 May 2023 14:28:01 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9887C63EDB; Wed, 17 May 2023 14:27:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8431DC433EF; Wed, 17 May 2023 14:27:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684333678; bh=AoRYpT1YLThZVxUKY9ePgVwDgIFfFF9PtIMUgbNxJyM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=WUNzXiJRJbiyKq9aqV065F7GO6DlSJst8wqq9ksE45jev/z5BNZjACDIsU+CKWd2p YgjGdckmgkxBOYGmqbO/Q7DmiNaGJvtGLhQQtz66t4NP/id+Uo0xu+RUnqZF6ypu4X FFZtvKjWpBheV7AQNhVpo1Hp4lfMZ0AfY5VaaItM8xbTSl5SojwEZIseT1QiWOyUeJ LeYUIrL2ez1oYq8e4XSBAeTn2ReJaBV8RZFEYpvmNI4OQI6b//O81nIMFwAGKKM2qy d5iS608w4+1IOEz2k6JrXrTWYbS3l4RsjV0WCQ9ODtU1E64QoYkryjNgQo+2XTUAyC R+33bZ4yB6rVg== Date: Wed, 17 May 2023 23:27:51 +0900 From: Masami Hiramatsu (Google) To: Ze Gao Cc: Steven Rostedt , Albert Ou , Alexander Gordeev , Alexei Starovoitov , Borislav Petkov , Christian Borntraeger , Dave Hansen , Heiko Carstens , "H. Peter Anvin" , Ingo Molnar , Palmer Dabbelt , Paul Walmsley , Sven Schnelle , Thomas Gleixner , Vasily Gorbik , x86@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Conor Dooley , Jiri Olsa , Yonghong Song , Ze Gao Subject: Re: [PATCH v3 2/4] fprobe: make fprobe_kprobe_handler recursion free Message-Id: <20230517232751.09126a6cec8786a954e54bcf@kernel.org> In-Reply-To: <20230517034510.15639-3-zegao@tencent.com> References: <20230517034510.15639-1-zegao@tencent.com> <20230517034510.15639-3-zegao@tencent.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230517_072759_480905_3E0CA983 X-CRM114-Status: GOOD ( 27.76 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Wed, 17 May 2023 11:45:07 +0800 Ze Gao wrote: > Current implementation calls kprobe related functions before doing > ftrace recursion check in fprobe_kprobe_handler, which opens door > to kernel crash due to stack recursion if preempt_count_{add, sub} > is traceable in kprobe_busy_{begin, end}. > > Things goes like this without this patch quoted from Steven: > " > fprobe_kprobe_handler() { > kprobe_busy_begin() { > preempt_disable() { > preempt_count_add() { <-- trace > fprobe_kprobe_handler() { > [ wash, rinse, repeat, CRASH!!! ] > " > > By refactoring the common part out of fprobe_kprobe_handler and > fprobe_handler and call ftrace recursion detection at the very beginning, > the whole fprobe_kprobe_handler is free from recursion. > > Signed-off-by: Ze Gao > Acked-by: Masami Hiramatsu (Google) > Link: https://lore.kernel.org/linux-trace-kernel/20230516071830.8190-3-zegao@tencent.com > --- > kernel/trace/fprobe.c | 59 ++++++++++++++++++++++++++++++++----------- > 1 file changed, 44 insertions(+), 15 deletions(-) > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > index 9abb3905bc8e..097c740799ba 100644 > --- a/kernel/trace/fprobe.c > +++ b/kernel/trace/fprobe.c > @@ -20,30 +20,22 @@ struct fprobe_rethook_node { > char data[]; > }; > > -static void fprobe_handler(unsigned long ip, unsigned long parent_ip, > - struct ftrace_ops *ops, struct ftrace_regs *fregs) > +static inline void __fprobe_handler(unsigned long ip, unsigned long > + parent_ip, struct ftrace_ops *ops, struct ftrace_regs *fregs) OK, I picked up this series to probes/fixes. Note that I fixed this line because the "unsigned long parent_ip" was split into 2 lines. Thank you, > { > struct fprobe_rethook_node *fpr; > struct rethook_node *rh = NULL; > struct fprobe *fp; > void *entry_data = NULL; > - int bit, ret; > + int ret; > > fp = container_of(ops, struct fprobe, ops); > - if (fprobe_disabled(fp)) > - return; > - > - bit = ftrace_test_recursion_trylock(ip, parent_ip); > - if (bit < 0) { > - fp->nmissed++; > - return; > - } > > if (fp->exit_handler) { > rh = rethook_try_get(fp->rethook); > if (!rh) { > fp->nmissed++; > - goto out; > + return; > } > fpr = container_of(rh, struct fprobe_rethook_node, node); > fpr->entry_ip = ip; > @@ -61,23 +53,60 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip, > else > rethook_hook(rh, ftrace_get_regs(fregs), true); > } > -out: > +} > + > +static void fprobe_handler(unsigned long ip, unsigned long parent_ip, > + struct ftrace_ops *ops, struct ftrace_regs *fregs) > +{ > + struct fprobe *fp; > + int bit; > + > + fp = container_of(ops, struct fprobe, ops); > + if (fprobe_disabled(fp)) > + return; > + > + /* recursion detection has to go before any traceable function and > + * all functions before this point should be marked as notrace > + */ > + bit = ftrace_test_recursion_trylock(ip, parent_ip); > + if (bit < 0) { > + fp->nmissed++; > + return; > + } > + __fprobe_handler(ip, parent_ip, ops, fregs); > ftrace_test_recursion_unlock(bit); > + > } > NOKPROBE_SYMBOL(fprobe_handler); > > static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *ops, struct ftrace_regs *fregs) > { > - struct fprobe *fp = container_of(ops, struct fprobe, ops); > + struct fprobe *fp; > + int bit; > + > + fp = container_of(ops, struct fprobe, ops); > + if (fprobe_disabled(fp)) > + return; > + > + /* recursion detection has to go before any traceable function and > + * all functions called before this point should be marked as notrace > + */ > + bit = ftrace_test_recursion_trylock(ip, parent_ip); > + if (bit < 0) { > + fp->nmissed++; > + return; > + } > > if (unlikely(kprobe_running())) { > fp->nmissed++; > return; > } > + > kprobe_busy_begin(); > - fprobe_handler(ip, parent_ip, ops, fregs); > + __fprobe_handler(ip, parent_ip, ops, fregs); > kprobe_busy_end(); > + ftrace_test_recursion_unlock(bit); > } > > static void fprobe_exit_handler(struct rethook_node *rh, void *data, > -- > 2.40.1 > -- Masami Hiramatsu (Google) _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv