From patchwork Mon Mar 29 07:56:43 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: 410816 Delivered-To: patch@linaro.org Received: by 2002:a02:8562:0:0:0:0:0 with SMTP id g89csp3433082jai; Mon, 29 Mar 2021 01:35:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz4k1GEZrmzUaVoExxOshMxlL1/bR3vCH5lh1R6XL3hGHMHO0iKSSsREEQULF67B55/AbOK X-Received: by 2002:aa7:d813:: with SMTP id v19mr27441417edq.213.1617006951971; Mon, 29 Mar 2021 01:35:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1617006951; cv=none; d=google.com; s=arc-20160816; b=Ey602DEqsaOt42h+rQbAlGfTA7ezvNd+Z88h9Jeyk+Dhln7UlmqBRm6G27O3wuW/HL dGZG51jqemd7PpweKU2XdFABZmpjvtQO6cUkfkufoKjGpAh/i8CIa+yqnAwGsVwvremQ 9FrBhG64CZYPyPQQOvR8mN0ft3r+77ibWl6xwynPPYRubr44mOgaKBJE+C226qbJQMhH JTkxT8Th5awi6qqjDunkwmsV85Gas3a0JEJpshTxhDN84gWH2rZ6EHr6iU0NA96LekSW zYhCFsStYnfPsn23DRSehblg0Rr2wj4LEUrogKrTYn6KhG6fCmTEtGXUec/w8Khr3qoO UPyw== 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=BgQnNsJeZXcMjJ5jdr4KANttL9UizwcbVsHB8I0jtvE/peB5pEr6YFVCmgsc6R0DqL 3Hzeb+Qry+s7A/lUd80W5Y8EL42m8v+WG4eYZcBqPr9MZZFWwRcU7V+Xvugg39YuNpUQ 5Xl0TsAWphs8RA9TOta6wzi6PNWEAslj3wO+F24E4STkdA0NGEVoG4XFWvmshLREgrf2 Kc8eQtE1f6uDvJnkbxZzym5KIQ3yodIGhb1tPvQp5aYudiTSMmqKqyBtpL7wH8dpaoqq sR35R6BncJs5FMSz03Z7qXsjvHNGf7FoPp1XewfiAogSrl4th/zYY4caTaB63Kf1T6H1 5lzg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=ecsN5oi8; 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 u10si7882036ejg.204.2021.03.29.01.35.51; Mon, 29 Mar 2021 01:35:51 -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=ecsN5oi8; 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 S233465AbhC2IfU (ORCPT + 12 others); Mon, 29 Mar 2021 04:35:20 -0400 Received: from mail.kernel.org ([198.145.29.99]:52660 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233613AbhC2IeI (ORCPT ); Mon, 29 Mar 2021 04:34:08 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id A40D061879; Mon, 29 Mar 2021 08:34:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1617006848; bh=kqLmg3L6lM80tubfHMERXklbLXUxYHXEpWR/kI0I+q0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ecsN5oi8z1j7dABsSzJF1rkzZ0bnLVaGEyuxSrbCNHiLxibTJMTumZqmOgmUazMds 8NTEMnYP1kxcrXVzjj4XqMenyQJnnT6SdYhtgF6S6/nZtyDFaj9OWtOm3ya7jOhUR9 8IWqx8UMP4BNAOzqudB/sV6MKTCr1BJreCwOr8ww= 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.11 087/254] arm64: stacktrace: dont trace arch_stack_walk() Date: Mon, 29 Mar 2021 09:56:43 +0200 Message-Id: <20210329075636.032542664@linuxfoundation.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210329075633.135869143@linuxfoundation.org> References: <20210329075633.135869143@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));