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 DA5F9C433EF for ; Fri, 11 Mar 2022 07:46:55 +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:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=EDQDAEr5T8vBwqHYWjQT0SDth37RpZ3E7tWrWtM9HhE=; b=x8R4QfHTUQ9k3X RrM0FlochhDdPzibISc7DtncxedfeH0EEVi0uODQNOb9otoKVsLMSsxISGCjD4O5rZ2tMHiubCamB y/v2IPG+YPlEBC5ieosUE8Eb3WO+hiqr7JHbjMf1Nfcgv5i9nx0LjUanA+waXhJ2zPWjqXvQEinUH uDMHe56eHK8uwqLsJQ3obF6hurtOjF4yM8HqBmc/uFAiKhXFTfJXhPNSFOpMjsaAbEGw7GrGNm2DL idAbWNwIaWJ5EU5fDe42+36cd3QTF8GjvJRzrHbhwhLPKpkchVyg5vyyn982TPXrZoS6NF/Q8YJ4O V3WybOFC6010ru3fSyEg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nSZy1-00FOEE-Hi; Fri, 11 Mar 2022 07:45:41 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nSZxx-00FODn-KT for linux-arm-kernel@lists.infradead.org; Fri, 11 Mar 2022 07:45:39 +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 ams.source.kernel.org (Postfix) with ESMTPS id 0418DB82AD0; Fri, 11 Mar 2022 07:45:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8AED7C340E9; Fri, 11 Mar 2022 07:45:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1646984734; bh=OgcAjpnkrI4Z5VN3tqdBOyNHYIf0TKW4dVETvD1s+zk=; h=From:To:Cc:Subject:Date:From; b=TvzYh2aDn+WEo6l8Q990SA4eEQGrhZvFmkIpCmmNh98Bs6xt8PCJa1j7GinrqRD6Z vRCylYxV/521qc4IMOU6HtUD0zxDrdPKta/wwZ5gFKu7bRRn+75qbD2iNk1yHwjTFe nPRL1q34KTr1lnSYR3kyk0uhUI7RREYUwbj7VkVIeJ20x4jT65pHlxBV4CC7XWYRrY ua1tLC+fmR6mzUXd38LXWdE8e57OOZa/oEWrHRL1WoD4BQ7T2sQtw0PflVZYgYst3J LNSGPP0vj/GCpQMVzSSGdhJE1DM1RSO9H3moUjoAGfVHpjaLMvfs87DUBVEJ+cVoUJ WF4HjJnFrChSg== From: Ard Biesheuvel To: linux@armlinux.org.uk Cc: naresh.kamboju@linaro.org, linux-arm-kernel@lists.infradead.org, Ard Biesheuvel Subject: [PATCH] ARM: unwind: only permit stack switch when unwinding call_with_stack() Date: Fri, 11 Mar 2022 08:45:29 +0100 Message-Id: <20220311074529.3254068-1-ardb@kernel.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=7137; h=from:subject; bh=OgcAjpnkrI4Z5VN3tqdBOyNHYIf0TKW4dVETvD1s+zk=; b=owEB7QES/pANAwAKAcNPIjmS2Y8kAcsmYgBiKv4YflwLh081KH1v6Qkke9eyCQjB0UScCNOBgGeL pbAL8o+JAbMEAAEKAB0WIQT72WJ8QGnJQhU3VynDTyI5ktmPJAUCYir+GAAKCRDDTyI5ktmPJOiVC/ 9ioiuRY2Z7fZPdfBSV6pnEbSrl0dHtKhE0kVDUvxBP3UQAxQE6vNoP+FPV86aOLlxaZ0m1eCUYpjxb QKFKr357Z+kArWAjgw4ome78aZ1HZpmmLArI3nBL2T1NdoJ79Jbw7eHa8CnJyJBugqO419vgF43xxT 4U5Nt7QiyZzYaYQGuhU0kY7Es8WkXSXDToqlDYOw0J6JsluqIWU8CzDXs1aVlRxYRgLsyaNakkCsLu w3HhkiKifOOlTl+/h7Y5CjCnpKWDzqUIlXP/Dl/a2THE0Htv2F0qeVfbc5cr5THECOPCVswXdXEjlR ntK9pYEIbAJqQMomWLHcZ6c12I08pcedsgv85032bYP/SjsAzlWmSQf2T1sdQB1afkRXHIrhcZioEt gn27roIOo2mNH0Bk2StMcq5ujfTXx+Gtp98PZdqax0PQcQqTF9zww7s0Tdqrvqedq/S87/79SO+DFr yNebi/kXOILgoj2bP5uSsUtg+rLys/mPi9yNKSluajklc= X-Developer-Key: i=ardb@kernel.org; a=openpgp; fpr=F43D03328115A198C90016883D200E9CA6329909 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220310_234538_001535_C5838078 X-CRM114-Status: GOOD ( 27.57 ) 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 Commit b6506981f880 ("ARM: unwind: support unwinding across multiple stacks") updated the logic in the ARM unwinder to widen the bounds within which SP is assumed to be valid, in order to allow the unwind to traverse from the IRQ stack to the task stack. This is necessary, as otherwise, unwinds started from the IRQ stack would terminate in the IRQ exception handler, making stacktraces substantially less useful. This turns out to be a mistake, as it breaks asynchronous unwinding across exceptions, when the exception is taken before the stack frame is consistent with the unwind info. For instance, in the following backtrace: ... generic_handle_arch_irq from call_with_stack+0x18/0x20 call_with_stack from __irq_svc+0x80/0x98 Exception stack(0xc7093e20 to 0xc7093e68) 3e20: b6a94a88 c7093ea0 00000008 00000000 c7093ea0 b7e127d0 00000051 c9220000 3e40: b6a94a88 b6a94a88 00000004 0002b000 0036b570 c7093e70 c040ca2c c0994a90 3e60: 20070013 ffffffff __irq_svc from __copy_to_user_std+0x20/0x378 ... we need to apply the following unwind directives: 0xc099720c <__copy_to_user_std+0x1c>: @0xc295d1d4 Compact model index: 1 0x9b vsp = r11 0xb1 0x0d pop {r0, r2, r3} 0x84 0x81 pop {r4, r11, r14} 0xb0 finish which tell us to switch to the frame pointer register R11 and proceed with the unwind from that. However, having been interrupted 0x20 bytes into the function: c09971f0 <__copy_to_user_std>: c09971f0: e59f3350 ldr r3, [pc, #848] c09971f4: e243c001 sub ip, r3, #1 c09971f8: e05cc000 subs ip, ip, r0 c09971fc: 228cc001 addcs ip, ip, #1 c0997200: 205cc002 subscs ip, ip, r2 c0997204: 33a00000 movcc r0, #0 c0997208: e320f014 csdb c099720c: e3a03000 mov r3, #0 c0997210: e92d481d push {r0, r2, r3, r4, fp, lr} c0997214: e1a0b00d mov fp, sp c0997218: e2522004 subs r2, r2, #4 the value for R11 recovered from the previous frame (__irq_svc) will be a snapshot of its value before the exception was taken (0x0002b000), which occurred at address __copy_to_user_std+0x20 (0xc0997210), when R11 had not been assigned its value yet. This means we can never assume that the SP values recovered from the stack or from the frame pointer are ever safe to use, given the need to do asynchronous unwinding, and the only robust approach is to revert to the previous approach, which is to derive bounds for SP based on the initial value, and never update them. We can make an exception, though: now that the IRQ stack switch is guaranteed to occur in call_with_stack(), we can implement a special case for this function, and use a different set of bounds based on the knowledge that it will always unwind from R11 rather than SP. As call_with_stack() is a hand-rolled assembly routine, this is guaranteed to remain that way. So let's do a partial revert of b6506981f880, and drop all manipulations for sp_low and sp_high based on the information collected during the unwind itself. To support call_with_stack(), set sp_low and sp_high explicitly to values derived from R11 when we unwind that function. The only downside is that, while unwinding an overflow of the vmap'ed stack will work fine as before, we will no longer be able to produce a backtrace that unwinds the overflow stack itself across the exception that was raised due to the faulting access to the guard region. However, this only affects exceptions caused by problems in the stack overflow handling code itself, in which case the remaining backtrace is not that relevant. Fixes: b6506981f880 ("ARM: unwind: support unwinding across multiple stacks") Signed-off-by: Ard Biesheuvel --- arch/arm/kernel/unwind.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c index e619ec7856b7..a37ea6c772cd 100644 --- a/arch/arm/kernel/unwind.c +++ b/arch/arm/kernel/unwind.c @@ -33,6 +33,8 @@ #include #include +#include "reboot.h" + /* Dummy functions to avoid linker complaints */ void __aeabi_unwind_cpp_pr0(void) { @@ -52,7 +54,6 @@ EXPORT_SYMBOL(__aeabi_unwind_cpp_pr2); struct unwind_ctrl_block { unsigned long vrs[16]; /* virtual register set */ const unsigned long *insn; /* pointer to the current instructions word */ - unsigned long sp_low; /* lowest value of sp allowed */ unsigned long sp_high; /* highest value of sp allowed */ unsigned long *lr_addr; /* address of LR value on the stack */ /* @@ -262,9 +263,6 @@ static int unwind_exec_pop_subset_r4_to_r13(struct unwind_ctrl_block *ctrl, } if (!load_sp) { ctrl->vrs[SP] = (unsigned long)vsp; - } else { - ctrl->sp_low = ctrl->vrs[SP]; - ctrl->sp_high = ALIGN(ctrl->sp_low, THREAD_SIZE); } return URC_OK; @@ -323,7 +321,6 @@ static int unwind_exec_insn(struct unwind_ctrl_block *ctrl) ctrl->vrs[SP] += ((insn & 0x3f) << 2) + 4; else if ((insn & 0xc0) == 0x40) { ctrl->vrs[SP] -= ((insn & 0x3f) << 2) + 4; - ctrl->sp_low = ctrl->vrs[SP]; } else if ((insn & 0xf0) == 0x80) { unsigned long mask; @@ -341,8 +338,6 @@ static int unwind_exec_insn(struct unwind_ctrl_block *ctrl) } else if ((insn & 0xf0) == 0x90 && (insn & 0x0d) != 0x0d) { ctrl->vrs[SP] = ctrl->vrs[insn & 0x0f]; - ctrl->sp_low = ctrl->vrs[SP]; - ctrl->sp_high = ALIGN(ctrl->sp_low, THREAD_SIZE); } else if ((insn & 0xf0) == 0xa0) { ret = unwind_exec_pop_r4_to_rN(ctrl, insn); if (ret) @@ -388,10 +383,11 @@ int unwind_frame(struct stackframe *frame) { const struct unwind_idx *idx; struct unwind_ctrl_block ctrl; + unsigned long sp_low; /* store the highest address on the stack to avoid crossing it*/ - ctrl.sp_low = frame->sp; - ctrl.sp_high = ALIGN(ctrl.sp_low - THREAD_SIZE, THREAD_ALIGN) + sp_low = frame->sp; + ctrl.sp_high = ALIGN(sp_low - THREAD_SIZE, THREAD_ALIGN) + THREAD_SIZE; pr_debug("%s(pc = %08lx lr = %08lx sp = %08lx)\n", __func__, @@ -452,6 +448,16 @@ int unwind_frame(struct stackframe *frame) ctrl.check_each_pop = 0; + if (prel31_to_addr(&idx->addr_offset) == (u32)&call_with_stack) { + /* + * call_with_stack() is the only place where we permit SP to + * jump from one stack to another, and since we know it is + * guaranteed to happen, set up the SP bounds accordingly. + */ + sp_low = frame->fp; + ctrl.sp_high = ALIGN(frame->fp, THREAD_SIZE); + } + while (ctrl.entries > 0) { int urc; if ((ctrl.sp_high - ctrl.vrs[SP]) < sizeof(ctrl.vrs)) @@ -459,7 +465,7 @@ int unwind_frame(struct stackframe *frame) urc = unwind_exec_insn(&ctrl); if (urc < 0) return urc; - if (ctrl.vrs[SP] < ctrl.sp_low || ctrl.vrs[SP] > ctrl.sp_high) + if (ctrl.vrs[SP] < sp_low || ctrl.vrs[SP] > ctrl.sp_high) return -URC_FAILURE; } -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel