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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B0DCC433F5 for ; Tue, 12 Oct 2021 14:18:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 53FE96101D for ; Tue, 12 Oct 2021 14:18:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236695AbhJLOUq (ORCPT ); Tue, 12 Oct 2021 10:20:46 -0400 Received: from mail.kernel.org ([198.145.29.99]:55198 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237248AbhJLOUm (ORCPT ); Tue, 12 Oct 2021 10:20:42 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0D4E560EFE; Tue, 12 Oct 2021 14:18:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1634048320; bh=7fvG1qwOsQbbh48aDMK9Imgg95CjCCaeJfTcDCkFPqk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=W7a5X2f1XjAD3IZvefCwbMGY529p2jXc61NYvsA1O6NIn3rjYC6qMGamPymdaNUyE mPPiNJhzLSc4QvcM38dcbpIAVJL1JUTps5ZSW2JChoHBkr+q5uQpDyWzvb9iSrZNcs no3AGukJynB6lCaWi4aVy3ufdKU20p/CdKpkEnfVRRfQeIkUPoZsIm6T0WX6g3itrY YBF6lsFDYacrClAu0w1Q9FbNXUC0mSQL3+FFs9vG1WaOLM/5sxFhpXcyetCy39RmGx agWesXqiVUbATsvKe+MgTy2DBQmpZKMZCKL9kUQ7KocWdWeE+UKLnDCh8iTDaqqQjQ WbwAYqLw/c3vA== Date: Tue, 12 Oct 2021 23:18:35 +0900 From: Masami Hiramatsu To: Nick Desaulniers Cc: Steven Rostedt , "Naveen N . Rao" , Ananth N Mavinakayanahalli , Ingo Molnar , linux-kernel@vger.kernel.org, Sven Schnelle , Catalin Marinas , Will Deacon , Russell King , Nathan Chancellor , linux-arm-kernel@lists.infradead.org, Nathan Huckleberry Subject: Re: [PATCH 6/8] ARM: clang: Do not relay on lr register for stacktrace Message-Id: <20211012231835.522ac7ba366e5019192c1a5a@kernel.org> In-Reply-To: References: <163369609308.636038.15295764725220907794.stgit@devnote2> <163369614818.636038.5019945597127474028.stgit@devnote2> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Nick, On Mon, 11 Oct 2021 11:45:22 -0700 Nick Desaulniers wrote: > On Fri, Oct 8, 2021 at 5:29 AM Masami Hiramatsu wrote: > > > > Currently the stacktrace on clang compiled arm kernel uses the 'lr' > > register to find the first frame address from pt_regs. However, that > > is wrong after calling another function, because the 'lr' register > > is used by 'bl' instruction and never be recovered. > > > > As same as gcc arm kernel, directly use the frame pointer (x11) of > > the pt_regs to find the first frame address. > > Hi Masami, > Thanks for the patch. Testing with ARCH=arm defconfig (multi_v7_defconfig) > > Before this patch: > > $ mount -t proc /proc > $ echo 0 > /proc/sys/kernel/kptr_restrict > $ cat /proc/self/stack > [<0>] proc_single_show+0x4c/0xb8 > [<0>] seq_read_iter+0x174/0x4d8 > [<0>] seq_read+0x134/0x158 > [<0>] vfs_read+0xcc/0x2f8 > [<0>] ksys_read+0x74/0xd0 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xbea38cc0 > > After this patch: > $ mount -t proc /proc > $ echo 0 > /proc/sys/kernel/kptr_restrict > $ cat /proc/self/stack > [<0>] proc_single_show+0x4c/0xb8 > [<0>] seq_read_iter+0x174/0x4d8 > [<0>] seq_read+0x134/0x158 > [<0>] vfs_read+0xcc/0x2f8 > [<0>] ksys_read+0x74/0xd0 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xbeb55cc0 > > Is there a different way to test/verify this patch? (I'm pretty sure > we had verified the WARN_ONCE functionality with this, too.) Hmm, I found this bug while testing my kretprobe-stacktrace test ([2/8] in this series), so if you apply this series and revert this patch and enable CONFIG_KPROBES_SANITY_TEST, you'll see that the tests failures as below. [ 4.062056] ok 4 - test_kretprobes [ 4.069944] # test_stacktrace_on_kretprobe: EXPECTATION FAILED at kernel/test_kprobes.c:235 [ 4.069944] Expected i != ret, but [ 4.069944] i == 10 [ 4.069944] ret == 10 [ 4.072692] not ok 5 - test_stacktrace_on_kretprobe [ 4.088171] # test_stacktrace_on_nested_kretprobe: EXPECTATION FAILED at kernel/test_kprobes.c:235 [ 4.088171] Expected i != ret, but [ 4.088171] i == 10 [ 4.088171] ret == 10 [ 4.091265] not ok 6 - test_stacktrace_on_nested_kretprobe This means the test failed to find the correct return address from the stacktrace. Applying this patch, [ 4.283953] ok 4 - test_kretprobes [ 4.290206] ok 5 - test_stacktrace_on_kretprobe [ 4.300429] ok 6 - test_stacktrace_on_nested_kretprobe [ 4.301743] # kprobes_test: pass:6 fail:0 skip:0 total:6 > > If I change from CONFIG_UNWINDER_ARM=y to > CONFIG_UNWINDER_FRAME_POINTER=y, before: > > # cat /proc/self/stack > [<0>] stack_trace_save_tsk+0x50/0x6c > [<0>] proc_pid_stack+0xa0/0xf8 > [<0>] proc_single_show+0x50/0xbc > [<0>] seq_read_iter+0x178/0x4ec > [<0>] seq_read+0x138/0x15c > [<0>] vfs_read+0xd0/0x304 > [<0>] ksys_read+0x78/0xd4 > [<0>] sys_read+0xc/0x10 > > after: > # cat /proc/self/stack > [<0>] proc_pid_stack+0xa0/0xf8 > [<0>] proc_single_show+0x50/0xbc > [<0>] seq_read_iter+0x178/0x4ec > [<0>] seq_read+0x138/0x15c > [<0>] vfs_read+0xd0/0x304 > [<0>] ksys_read+0x78/0xd4 > [<0>] sys_read+0xc/0x10 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xffffffff > > So I guess this helps the CONFIG_UNWINDER_FRAME_POINTER=y case? (That > final frame address looks wrong, but is potentially yet another bug; > perhaps for clang we need to manually store the previous frame's pc at > a different offset before jumping to __entry_text_start). Ah, yes. I didn't touch the UNWINDER_ARM. As you may know the unwind_frame()@arch/arm/kernel/stacktrace.c is compiled only CONFIG_UNWINDER_FRAME_POINTER=y. > > Also, I'm curious about CONFIG_THUMB2_KERNEL (forces CONFIG_UNWINDER_ARM=y). > > before: > # cat /proc/self/stack > [<0>] proc_single_show+0x31/0x86 > [<0>] seq_read_iter+0xff/0x326 > [<0>] seq_read+0xd7/0xf2 > [<0>] vfs_read+0x93/0x20e > [<0>] ksys_read+0x53/0x92 > [<0>] ret_fast_syscall+0x1/0x52 > [<0>] 0xbe9a9cc0 > > after: > # cat /proc/self/stack > [<0>] proc_single_show+0x31/0x86 > [<0>] seq_read_iter+0xff/0x326 > [<0>] seq_read+0xd7/0xf2 > [<0>] vfs_read+0x93/0x20e > [<0>] ksys_read+0x53/0x92 > [<0>] ret_fast_syscall+0x1/0x52 > [<0>] 0xbec08cc0 > > Tested-by: Nick Desaulniers > > so likely this fixes/improves CONFIG_UNWINDER_FRAME_POINTER=y? Is that correct? Yes, that is correct. Thank you! > > > > > Signed-off-by: Masami Hiramatsu > > --- > > arch/arm/kernel/stacktrace.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c > > index 76ea4178a55c..db798eac7431 100644 > > --- a/arch/arm/kernel/stacktrace.c > > +++ b/arch/arm/kernel/stacktrace.c > > @@ -54,8 +54,7 @@ int notrace unwind_frame(struct stackframe *frame) > > > > frame->sp = frame->fp; > > frame->fp = *(unsigned long *)(fp); > > - frame->pc = frame->lr; > > - frame->lr = *(unsigned long *)(fp + 4); > > + frame->pc = *(unsigned long *)(fp + 4); > > #else > > /* check current frame pointer is within bounds */ > > if (fp < low + 12 || fp > high - 4) > > > > -- > Thanks, > ~Nick Desaulniers -- Masami Hiramatsu 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0FF5C433F5 for ; Tue, 12 Oct 2021 14:21:20 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id B981E608FE for ; Tue, 12 Oct 2021 14:21:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B981E608FE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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=SXogWD+vwROxO4eOSmUgO8c8JJj6XfSBrmsyr3FPFJM=; b=B6j17OjljHLgAE lujWrPdZ4alzpbAbr6R0pkoTgfaiPXG2rL+OGYAJ5mjy1EJ05ocftPBdGuQHKojaVTx/maKkaOrvx 4agQB6/VQqmGCcKQA1Y6jmGRBuzg9+iiQ2W9KWlcCIhz0ML+UMOqFill8cI5qjAhykQa4aUqs+8PW bL34KhrSYMi1lfPH12s16Vzk5vY7mxEqaBOHi+TGZFEIGhC6w6uHjf3ZXCbOh3RYWVwjHv5lb/DuF 63nF4kRr5z/ea023YalP992s1Ojd5BGAdIkgVYsMZSDvn1OToImGYt0Hbd+sbA7a6XD7Wx63dRaxB O/cD7WylAzEEPzKRKvwQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maIcA-00DAOS-52; Tue, 12 Oct 2021 14:18:46 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1maIc5-00DANW-Jx for linux-arm-kernel@lists.infradead.org; Tue, 12 Oct 2021 14:18:43 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0D4E560EFE; Tue, 12 Oct 2021 14:18:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1634048320; bh=7fvG1qwOsQbbh48aDMK9Imgg95CjCCaeJfTcDCkFPqk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=W7a5X2f1XjAD3IZvefCwbMGY529p2jXc61NYvsA1O6NIn3rjYC6qMGamPymdaNUyE mPPiNJhzLSc4QvcM38dcbpIAVJL1JUTps5ZSW2JChoHBkr+q5uQpDyWzvb9iSrZNcs no3AGukJynB6lCaWi4aVy3ufdKU20p/CdKpkEnfVRRfQeIkUPoZsIm6T0WX6g3itrY YBF6lsFDYacrClAu0w1Q9FbNXUC0mSQL3+FFs9vG1WaOLM/5sxFhpXcyetCy39RmGx agWesXqiVUbATsvKe+MgTy2DBQmpZKMZCKL9kUQ7KocWdWeE+UKLnDCh8iTDaqqQjQ WbwAYqLw/c3vA== Date: Tue, 12 Oct 2021 23:18:35 +0900 From: Masami Hiramatsu To: Nick Desaulniers Cc: Steven Rostedt , "Naveen N . Rao" , Ananth N Mavinakayanahalli , Ingo Molnar , linux-kernel@vger.kernel.org, Sven Schnelle , Catalin Marinas , Will Deacon , Russell King , Nathan Chancellor , linux-arm-kernel@lists.infradead.org, Nathan Huckleberry Subject: Re: [PATCH 6/8] ARM: clang: Do not relay on lr register for stacktrace Message-Id: <20211012231835.522ac7ba366e5019192c1a5a@kernel.org> In-Reply-To: References: <163369609308.636038.15295764725220907794.stgit@devnote2> <163369614818.636038.5019945597127474028.stgit@devnote2> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; 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-20211012_071841_727298_1730D27C X-CRM114-Status: GOOD ( 29.90 ) X-BeenThere: linux-arm-kernel@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-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Nick, On Mon, 11 Oct 2021 11:45:22 -0700 Nick Desaulniers wrote: > On Fri, Oct 8, 2021 at 5:29 AM Masami Hiramatsu wrote: > > > > Currently the stacktrace on clang compiled arm kernel uses the 'lr' > > register to find the first frame address from pt_regs. However, that > > is wrong after calling another function, because the 'lr' register > > is used by 'bl' instruction and never be recovered. > > > > As same as gcc arm kernel, directly use the frame pointer (x11) of > > the pt_regs to find the first frame address. > > Hi Masami, > Thanks for the patch. Testing with ARCH=arm defconfig (multi_v7_defconfig) > > Before this patch: > > $ mount -t proc /proc > $ echo 0 > /proc/sys/kernel/kptr_restrict > $ cat /proc/self/stack > [<0>] proc_single_show+0x4c/0xb8 > [<0>] seq_read_iter+0x174/0x4d8 > [<0>] seq_read+0x134/0x158 > [<0>] vfs_read+0xcc/0x2f8 > [<0>] ksys_read+0x74/0xd0 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xbea38cc0 > > After this patch: > $ mount -t proc /proc > $ echo 0 > /proc/sys/kernel/kptr_restrict > $ cat /proc/self/stack > [<0>] proc_single_show+0x4c/0xb8 > [<0>] seq_read_iter+0x174/0x4d8 > [<0>] seq_read+0x134/0x158 > [<0>] vfs_read+0xcc/0x2f8 > [<0>] ksys_read+0x74/0xd0 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xbeb55cc0 > > Is there a different way to test/verify this patch? (I'm pretty sure > we had verified the WARN_ONCE functionality with this, too.) Hmm, I found this bug while testing my kretprobe-stacktrace test ([2/8] in this series), so if you apply this series and revert this patch and enable CONFIG_KPROBES_SANITY_TEST, you'll see that the tests failures as below. [ 4.062056] ok 4 - test_kretprobes [ 4.069944] # test_stacktrace_on_kretprobe: EXPECTATION FAILED at kernel/test_kprobes.c:235 [ 4.069944] Expected i != ret, but [ 4.069944] i == 10 [ 4.069944] ret == 10 [ 4.072692] not ok 5 - test_stacktrace_on_kretprobe [ 4.088171] # test_stacktrace_on_nested_kretprobe: EXPECTATION FAILED at kernel/test_kprobes.c:235 [ 4.088171] Expected i != ret, but [ 4.088171] i == 10 [ 4.088171] ret == 10 [ 4.091265] not ok 6 - test_stacktrace_on_nested_kretprobe This means the test failed to find the correct return address from the stacktrace. Applying this patch, [ 4.283953] ok 4 - test_kretprobes [ 4.290206] ok 5 - test_stacktrace_on_kretprobe [ 4.300429] ok 6 - test_stacktrace_on_nested_kretprobe [ 4.301743] # kprobes_test: pass:6 fail:0 skip:0 total:6 > > If I change from CONFIG_UNWINDER_ARM=y to > CONFIG_UNWINDER_FRAME_POINTER=y, before: > > # cat /proc/self/stack > [<0>] stack_trace_save_tsk+0x50/0x6c > [<0>] proc_pid_stack+0xa0/0xf8 > [<0>] proc_single_show+0x50/0xbc > [<0>] seq_read_iter+0x178/0x4ec > [<0>] seq_read+0x138/0x15c > [<0>] vfs_read+0xd0/0x304 > [<0>] ksys_read+0x78/0xd4 > [<0>] sys_read+0xc/0x10 > > after: > # cat /proc/self/stack > [<0>] proc_pid_stack+0xa0/0xf8 > [<0>] proc_single_show+0x50/0xbc > [<0>] seq_read_iter+0x178/0x4ec > [<0>] seq_read+0x138/0x15c > [<0>] vfs_read+0xd0/0x304 > [<0>] ksys_read+0x78/0xd4 > [<0>] sys_read+0xc/0x10 > [<0>] __entry_text_start+0x14/0x14 > [<0>] 0xffffffff > > So I guess this helps the CONFIG_UNWINDER_FRAME_POINTER=y case? (That > final frame address looks wrong, but is potentially yet another bug; > perhaps for clang we need to manually store the previous frame's pc at > a different offset before jumping to __entry_text_start). Ah, yes. I didn't touch the UNWINDER_ARM. As you may know the unwind_frame()@arch/arm/kernel/stacktrace.c is compiled only CONFIG_UNWINDER_FRAME_POINTER=y. > > Also, I'm curious about CONFIG_THUMB2_KERNEL (forces CONFIG_UNWINDER_ARM=y). > > before: > # cat /proc/self/stack > [<0>] proc_single_show+0x31/0x86 > [<0>] seq_read_iter+0xff/0x326 > [<0>] seq_read+0xd7/0xf2 > [<0>] vfs_read+0x93/0x20e > [<0>] ksys_read+0x53/0x92 > [<0>] ret_fast_syscall+0x1/0x52 > [<0>] 0xbe9a9cc0 > > after: > # cat /proc/self/stack > [<0>] proc_single_show+0x31/0x86 > [<0>] seq_read_iter+0xff/0x326 > [<0>] seq_read+0xd7/0xf2 > [<0>] vfs_read+0x93/0x20e > [<0>] ksys_read+0x53/0x92 > [<0>] ret_fast_syscall+0x1/0x52 > [<0>] 0xbec08cc0 > > Tested-by: Nick Desaulniers > > so likely this fixes/improves CONFIG_UNWINDER_FRAME_POINTER=y? Is that correct? Yes, that is correct. Thank you! > > > > > Signed-off-by: Masami Hiramatsu > > --- > > arch/arm/kernel/stacktrace.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c > > index 76ea4178a55c..db798eac7431 100644 > > --- a/arch/arm/kernel/stacktrace.c > > +++ b/arch/arm/kernel/stacktrace.c > > @@ -54,8 +54,7 @@ int notrace unwind_frame(struct stackframe *frame) > > > > frame->sp = frame->fp; > > frame->fp = *(unsigned long *)(fp); > > - frame->pc = frame->lr; > > - frame->lr = *(unsigned long *)(fp + 4); > > + frame->pc = *(unsigned long *)(fp + 4); > > #else > > /* check current frame pointer is within bounds */ > > if (fp < low + 12 || fp > high - 4) > > > > -- > Thanks, > ~Nick Desaulniers -- Masami Hiramatsu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel