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 48353C433F5 for ; Wed, 10 Nov 2021 03:15:27 +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 0109860E53 for ; Wed, 10 Nov 2021 03:15:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0109860E53 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.microsoft.com 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:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=yKtvrYXEA0TG/TLPlPRbKExDuPskwRCqHYtSEkyxXFc=; b=rjCECDLwwWXjvmnIUPzSrVgi/B LkCEb8SS2tlrW7nVQI+4l6xU07rIZrzn19NddVGFXi9jh0LhUiboYBZXN+UZR5BUcqiOK6Epf4TTC Xmn8NI1tL8Iw+/OEjgNXTexhsUy6iA6zbHVf9L+6MkzaV5D/u2D4uwio4/fTz12k8NtfQVm6Fuw3E zrmaKpR0KIX5YBvXhXqMc/GcN3E3PiLK6LXsmgUeZxcUvtV9QYYSOKymCWyd3y3qkjiBMBQnnrDAN 4YO6XPAhvDgURIq3eXg85HdKWOPOXfoGY2GxupZbyAv1A/pJJH+DFYRaJ+UZF8/k0i1lbfe5gxpjQ 1YHvG/gQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mke3l-004NKJ-Dp; Wed, 10 Nov 2021 03:14:01 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mke3h-004NJb-Km for linux-arm-kernel@lists.infradead.org; Wed, 10 Nov 2021 03:13:59 +0000 Received: from [192.168.254.32] (unknown [47.187.212.181]) by linux.microsoft.com (Postfix) with ESMTPSA id 2DE1F20C3535; Tue, 9 Nov 2021 19:13:54 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 2DE1F20C3535 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1636514034; bh=JYjxViWkrGsEDHLHlUlTuhozjv8AcwtqMtnbcBM6dOI=; h=Subject:To:References:From:Date:In-Reply-To:From; b=WT85f9ixWA4Hah3zzTkyHEz0QcK2br5nEir/6JXZ32/8NSNqBjxL6FGqGPXiLJ1iP uGl9naxjuTBPxKLLGlPz4SZ/FfiirbauPgXK9tzHsjw+yQ58IbwVeZkTXPUpwjnlyX HU1yH71Ln30Q4SrGxIV9JLynwCdITGjrrGN39SWM= Subject: Re: [PATCH v10 10/11] arm64: Introduce stack trace reliability checks in the unwinder To: "nobuta.keiya@fujitsu.com" , "mark.rutland@arm.com" , "broonie@kernel.org" , "jpoimboe@redhat.com" , "ardb@kernel.org" , "sjitindarsingh@gmail.com" , "catalin.marinas@arm.com" , "will@kernel.org" , "jmorris@namei.org" , "linux-arm-kernel@lists.infradead.org" , "live-patching@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <20211015025847.17694-1-madvenka@linux.microsoft.com> <20211015025847.17694-11-madvenka@linux.microsoft.com> From: "Madhavan T. Venkataraman" Message-ID: <89ec9563-1484-af7d-6d9f-7ba8e01d5a27@linux.microsoft.com> Date: Tue, 9 Nov 2021 21:13:53 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211109_191357_793381_1D23EC54 X-CRM114-Status: GOOD ( 33.06 ) 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="iso-2022-jp" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Nobuta, Sorry for the delay in responding to your comment. I will fix the issue you have raised in the next version. Thanks. Again, sorry for the late response. Madhavan On 11/4/21 7:39 AM, nobuta.keiya@fujitsu.com wrote: > Hi Madhavan, > >> -----Original Message----- >> From: madvenka@linux.microsoft.com >> Sent: Friday, October 15, 2021 11:59 AM >> To: mark.rutland@arm.com; broonie@kernel.org; jpoimboe@redhat.com; ardb@kernel.org; Nobuta, Keiya/信田 圭哉 >> ; sjitindarsingh@gmail.com; catalin.marinas@arm.com; will@kernel.org; jmorris@namei.org; >> linux-arm-kernel@lists.infradead.org; live-patching@vger.kernel.org; linux-kernel@vger.kernel.org; >> madvenka@linux.microsoft.com >> Subject: [PATCH v10 10/11] arm64: Introduce stack trace reliability checks in the unwinder >> >> From: "Madhavan T. Venkataraman" >> >> There are some kernel features and conditions that make a stack trace unreliable. Callers may require the unwinder to detect >> these cases. >> E.g., livepatch. >> >> Introduce a new function called unwind_check_reliability() that will detect these cases and set a flag in the stack frame. Call >> unwind_check_reliability() for every frame, that is, in unwind_start() and unwind_next(). >> >> Introduce the first reliability check in unwind_check_reliability() - If a return PC is not a valid kernel text address, consider the >> stack trace unreliable. It could be some generated code. Other reliability checks will be added in the future. >> >> Let unwind() return a boolean to indicate if the stack trace is reliable. >> >> Introduce arch_stack_walk_reliable() for ARM64. This works like >> arch_stack_walk() except that it returns -EINVAL if the stack trace is not reliable. >> >> Until all the reliability checks are in place, arch_stack_walk_reliable() may not be used by livepatch. But it may be used by >> debug and test code. >> >> Signed-off-by: Madhavan T. Venkataraman >> --- >> arch/arm64/include/asm/stacktrace.h | 3 ++ >> arch/arm64/kernel/stacktrace.c | 48 ++++++++++++++++++++++++++++- >> 2 files changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h >> index ba2180c7d5cd..ce0710fa3037 100644 >> --- a/arch/arm64/include/asm/stacktrace.h >> +++ b/arch/arm64/include/asm/stacktrace.h >> @@ -51,6 +51,8 @@ struct stack_info { >> * replacement lr value in the ftrace graph stack. >> * >> * @failed: Unwind failed. >> + * >> + * @reliable: Stack trace is reliable. >> */ >> struct stackframe { >> unsigned long fp; >> @@ -62,6 +64,7 @@ struct stackframe { >> int graph; >> #endif >> bool failed; >> + bool reliable; >> }; >> >> extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, diff --git a/arch/arm64/kernel/stacktrace.c >> b/arch/arm64/kernel/stacktrace.c index 8e9e6f38c975..142f08ae515f 100644 >> --- a/arch/arm64/kernel/stacktrace.c >> +++ b/arch/arm64/kernel/stacktrace.c >> @@ -18,6 +18,22 @@ >> #include >> #include >> >> +/* >> + * Check the stack frame for conditions that make further unwinding unreliable. >> + */ >> +static void notrace unwind_check_reliability(struct stackframe *frame) >> +{ >> + /* >> + * If the PC is not a known kernel text address, then we cannot >> + * be sure that a subsequent unwind will be reliable, as we >> + * don't know that the code follows our unwind requirements. >> + */ >> + if (!__kernel_text_address(frame->pc)) >> + frame->reliable = false; >> +} >> + >> +NOKPROBE_SYMBOL(unwind_check_reliability); >> + >> /* >> * AArch64 PCS assigns the frame pointer to x29. >> * >> @@ -55,6 +71,8 @@ static void notrace unwind_start(struct stackframe *frame, unsigned long fp, >> frame->prev_fp = 0; >> frame->prev_type = STACK_TYPE_UNKNOWN; >> frame->failed = false; >> + frame->reliable = true; >> + unwind_check_reliability(frame); >> } >> >> NOKPROBE_SYMBOL(unwind_start); >> @@ -138,6 +156,7 @@ static void notrace unwind_next(struct task_struct *tsk, #endif /* >> CONFIG_FUNCTION_GRAPH_TRACER */ >> >> frame->pc = ptrauth_strip_insn_pac(frame->pc); >> + unwind_check_reliability(frame); >> } > > Isn't it necessary to check "final frame" before unwind_check_reliability()? > The frame at this point is unwound frame, so may be last frame. > > Or if move unwind_check_reliability() into unwind(), I think unwind() can > be twins as below: > > ~~~~~~~~ > unwind(...) { > <...> > for (unwind_start(...); unwind_continue(...); unwind_next(...)) > unwind_check_reliability(&frame); > } > > unwind_reliable(...) { > <...> > for (unwind_start(...); unwind_continue(...); unwind_next(...)) { > unwind_check_reliability(&frame); > if (!frame.reliable) > break; > } > > return (frame.reliable && !frame.failed); > } > ~~~~~~~~ > > > > Thanks, > Keiya > > >> >> NOKPROBE_SYMBOL(unwind_next); >> @@ -167,7 +186,7 @@ static bool notrace unwind_continue(struct task_struct *task, >> >> NOKPROBE_SYMBOL(unwind_continue); >> >> -static void notrace unwind(struct task_struct *tsk, >> +static bool notrace unwind(struct task_struct *tsk, >> unsigned long fp, unsigned long pc, >> bool (*fn)(void *, unsigned long), >> void *data) >> @@ -177,6 +196,7 @@ static void notrace unwind(struct task_struct *tsk, >> unwind_start(&frame, fp, pc); >> while (unwind_continue(tsk, &frame, fn, data)) >> unwind_next(tsk, &frame); >> + return frame.reliable; >> } >> >> NOKPROBE_SYMBOL(unwind); >> @@ -238,4 +258,30 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry, >> >> } >> >> +/* >> + * arch_stack_walk_reliable() may not be used for livepatch until all >> +of >> + * the reliability checks are in place in unwind_consume(). However, >> + * debug and test code can choose to use it even if all the checks are >> +not >> + * in place. >> + */ >> +noinline int notrace arch_stack_walk_reliable(stack_trace_consume_fn consume_fn, >> + void *cookie, >> + struct task_struct *task) >> +{ >> + unsigned long fp, pc; >> + >> + if (task == current) { >> + /* Skip arch_stack_walk_reliable() in the stack trace. */ >> + fp = (unsigned long)__builtin_frame_address(1); >> + pc = (unsigned long)__builtin_return_address(0); >> + } else { >> + /* Caller guarantees that the task is not running. */ >> + fp = thread_saved_fp(task); >> + pc = thread_saved_pc(task); >> + } >> + if (unwind(task, fp, pc, consume_fn, cookie)) >> + return 0; >> + return -EINVAL; >> +} >> + >> #endif >> -- >> 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel