From patchwork Mon Mar 29 07:56:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kroah-Hartman X-Patchwork-Id: 410805 Delivered-To: patch@linaro.org Received: by 2002:a02:8562:0:0:0:0:0 with SMTP id g89csp3425621jai; Mon, 29 Mar 2021 01:22:05 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzG5KSyMwS1kGu0izw3IXfayiIGIY4yDDrh4DQF28hf9OYJmgqxwaJINx1OOBNEXinSmzZA X-Received: by 2002:aa7:c88e:: with SMTP id p14mr26948549eds.119.1617006042504; Mon, 29 Mar 2021 01:20:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1617006042; cv=none; d=google.com; s=arc-20160816; b=vup4VQujY0b2pXEJ24HVs8/lXVuEiS09PuCBegVbOwsNopuxhVACM4/688J35Gd2gh jLtEsCBlIM4DQlvtDEYzzanNBMzXV9d7hVH6iYelyKANAK9QUNI3MThdYxeQZLq4QvB7 MSj8+5wbXndiMZX4kIueHS7wrlHLO5XF9p78Su2h2MmyCPYJ0/ZMh2kOnFNQMsTtIoLI L896MdYEOSQAagy5/z1og428Wt7ORmO3p2bOAeM4tdXcjSRfeEAS6yvo7p4cYzT8MaLG 4q4jwm36s6IGEcMYGle7o84OzSBiKNmpnbiO7THkBqEbSZb9g3y/FWPzByqrt1WiriVt l4gw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=ZJrHu94P1nrUxgsGHRm+33c2kqR7U0+7DhcJbFVaPsE=; b=J87mwAhw8MYZ/sNt5Fns2ULNDF8Bu+Z37tKAsVHLxdUJQc9p4raZ58ySSa6EJdhMGQ ylrgEN5tTfVDTvcP3DoScXdER9ab8qLH4c02Xt/nr/ldN30fv32hnZgQ4Q2R/rgcZfSr ju64jKaC89Clm/6VCC1AtQ973VjdqkSk5ZiF+h16jtoaQ9rr4k7QIrb6tNRJ1qarhQp3 wuUUPOeXUvHznn9iOUefSB1iOzMS2JG5B3X+MEGQXC7OoB/Y2tzNmLgAIf4Gyz6q9prQ ngTTSVeIAfPWn6JNhsJqi1g1a87TR9oCCP4PgC+0QXCQ67pFUHe9cRbySQBMpdXc9MnG iiRQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=bDv5HsHL; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j19si11781034edp.531.2021.03.29.01.20.42; Mon, 29 Mar 2021 01:20:42 -0700 (PDT) Received-SPF: pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=bDv5HsHL; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233289AbhC2IUM (ORCPT + 12 others); Mon, 29 Mar 2021 04:20:12 -0400 Received: from mail.kernel.org ([198.145.29.99]:36658 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233305AbhC2ITo (ORCPT ); Mon, 29 Mar 2021 04:19:44 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id A980461601; Mon, 29 Mar 2021 08:19:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1617005984; bh=kqLmg3L6lM80tubfHMERXklbLXUxYHXEpWR/kI0I+q0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bDv5HsHLe4E1VQlqI1Fo/VzTVOkBhmqE++fE9Y+AqmqlxhR4leQ3z+10zdVexg7fe VG7YTHIB0gaosgwiFhsMK9/intsFhdBCEA+jpboBoD8IgDZhPbGnEYRU+mFUOytVZ0 lKSbxRiirjtvU7vzVsuF1gWCDYx//FKYlfuHx0+w= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Mark Rutland , Catalin Marinas , Chen Jun , Marco Elver , Mark Brown , Will Deacon Subject: [PATCH 5.10 076/221] arm64: stacktrace: dont trace arch_stack_walk() Date: Mon, 29 Mar 2021 09:56:47 +0200 Message-Id: <20210329075631.725217944@linuxfoundation.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210329075629.172032742@linuxfoundation.org> References: <20210329075629.172032742@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Mark Rutland commit c607ab4f916d4d5259072eca34055d3f5a795c21 upstream. We recently converted arm64 to use arch_stack_walk() in commit: 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") The core stacktrace code expects that (when tracing the current task) arch_stack_walk() starts a trace at its caller, and does not include itself in the trace. However, arm64's arch_stack_walk() includes itself, and so traces include one more entry than callers expect. The core stacktrace code which calls arch_stack_walk() tries to skip a number of entries to prevent itself appearing in a trace, and the additional entry prevents skipping one of the core stacktrace functions, leaving this in the trace unexpectedly. We can fix this by having arm64's arch_stack_walk() begin the trace with its caller. The first value returned by the trace will be __builtin_return_address(0), i.e. the caller of arch_stack_walk(). The first frame record to be unwound will be __builtin_frame_address(1), i.e. the caller's frame record. To prevent surprises, arch_stack_walk() is also marked noinline. While __builtin_frame_address(1) is not safe in portable code, local GCC developers have confirmed that it is safe on arm64. To find the caller's frame record, the builtin can safely dereference the current function's frame record or (in theory) could stash the original FP into another GPR at function entry time, neither of which are problematic. Prior to this patch, the tracing code would unexpectedly show up in traces of the current task, e.g. | # cat /proc/self/stack | [<0>] stack_trace_save_tsk+0x98/0x100 | [<0>] proc_pid_stack+0xb4/0x130 | [<0>] proc_single_show+0x60/0x110 | [<0>] seq_read_iter+0x230/0x4d0 | [<0>] seq_read+0xdc/0x130 | [<0>] vfs_read+0xac/0x1e0 | [<0>] ksys_read+0x6c/0xfc | [<0>] __arm64_sys_read+0x20/0x30 | [<0>] el0_svc_common.constprop.0+0x60/0x120 | [<0>] do_el0_svc+0x24/0x90 | [<0>] el0_svc+0x2c/0x54 | [<0>] el0_sync_handler+0x1a4/0x1b0 | [<0>] el0_sync+0x170/0x180 After this patch, the tracing code will not show up in such traces: | # cat /proc/self/stack | [<0>] proc_pid_stack+0xb4/0x130 | [<0>] proc_single_show+0x60/0x110 | [<0>] seq_read_iter+0x230/0x4d0 | [<0>] seq_read+0xdc/0x130 | [<0>] vfs_read+0xac/0x1e0 | [<0>] ksys_read+0x6c/0xfc | [<0>] __arm64_sys_read+0x20/0x30 | [<0>] el0_svc_common.constprop.0+0x60/0x120 | [<0>] do_el0_svc+0x24/0x90 | [<0>] el0_svc+0x2c/0x54 | [<0>] el0_sync_handler+0x1a4/0x1b0 | [<0>] el0_sync+0x170/0x180 Erring on the side of caution, I've given this a spin with a bunch of toolchains, verifying the output of /proc/self/stack and checking that the assembly looked sound. For GCC (where we require version 5.1.0 or later) I tested with the kernel.org crosstool binares for versions 5.5.0, 6.4.0, 6.5.0, 7.3.0, 7.5.0, 8.1.0, 8.3.0, 8.4.0, 9.2.0, and 10.1.0. For clang (where we require version 10.0.1 or later) I tested with the llvm.org binary releases of 11.0.0, and 11.0.1. Fixes: 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK") Signed-off-by: Mark Rutland Cc: Catalin Marinas Cc: Chen Jun Cc: Marco Elver Cc: Mark Brown Cc: Will Deacon Cc: # 5.10.x Reviewed-by: Catalin Marinas Reviewed-by: Mark Brown Link: https://lore.kernel.org/r/20210319184106.5688-1-mark.rutland@arm.com Signed-off-by: Will Deacon Signed-off-by: Greg Kroah-Hartman --- arch/arm64/kernel/stacktrace.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -199,8 +199,9 @@ void show_stack(struct task_struct *tsk, #ifdef CONFIG_STACKTRACE -void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, - struct task_struct *task, struct pt_regs *regs) +noinline void arch_stack_walk(stack_trace_consume_fn consume_entry, + void *cookie, struct task_struct *task, + struct pt_regs *regs) { struct stackframe frame; @@ -208,8 +209,8 @@ void arch_stack_walk(stack_trace_consume start_backtrace(&frame, regs->regs[29], regs->pc); else if (task == current) start_backtrace(&frame, - (unsigned long)__builtin_frame_address(0), - (unsigned long)arch_stack_walk); + (unsigned long)__builtin_frame_address(1), + (unsigned long)__builtin_return_address(0)); else start_backtrace(&frame, thread_saved_fp(task), thread_saved_pc(task));