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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1595EC54E58 for ; Wed, 20 Mar 2024 19:41:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 71D8B6B0087; Wed, 20 Mar 2024 15:41:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6CCA76B0089; Wed, 20 Mar 2024 15:41:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5941C6B0095; Wed, 20 Mar 2024 15:41:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 4804B6B0087 for ; Wed, 20 Mar 2024 15:41:15 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id BBF6CA0D82 for ; Wed, 20 Mar 2024 19:41:14 +0000 (UTC) X-FDA: 81918436068.06.E6ACFDC Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [78.32.30.218]) by imf21.hostedemail.com (Postfix) with ESMTP id 7C7D31C0018 for ; Wed, 20 Mar 2024 19:41:11 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=armlinux.org.uk header.s=pandora-2019 header.b=vyT9UD0c; dmarc=pass (policy=none) header.from=armlinux.org.uk; spf=none (imf21.hostedemail.com: domain of "linux+linux-mm=kvack.org@armlinux.org.uk" has no SPF policy when checking 78.32.30.218) smtp.mailfrom="linux+linux-mm=kvack.org@armlinux.org.uk" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710963671; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=93N1WsDx3xkI8PIGtHQm769e5Sqg9ilfdoc7k8pVcyA=; b=kufec9W+LOSwsBvWK6otVQ57VUWzIt6vF1HQU+baJpzMwFCGS26eqBBsaK+bOmZ8mH5js0 gjsE19FgpXRS7eBzSWeismkzbrmxHgk6R2nDkA0XQoNDq5rUGLon4xnTWoni2pNhBsSZqc 53gTiCcFd0NeTg9lf6f7hWev4KqpSL0= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=armlinux.org.uk header.s=pandora-2019 header.b=vyT9UD0c; dmarc=pass (policy=none) header.from=armlinux.org.uk; spf=none (imf21.hostedemail.com: domain of "linux+linux-mm=kvack.org@armlinux.org.uk" has no SPF policy when checking 78.32.30.218) smtp.mailfrom="linux+linux-mm=kvack.org@armlinux.org.uk" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710963671; a=rsa-sha256; cv=none; b=BWoxsDsJJef5TjPV4kzPLe/dpwTyS0yvCFenzgTTE2DGukyLVtbgTvcXltlA3qaC+XGwLw t6kxhwifKgVlR8H8sMu5msPzZN9gaQntwd8QsAPq54tIxjYgxcu1MYFyqNMLdMC9fBE2Vs /LymQLYeIiVnV9PYwEdsJ4wFkZ31Pso= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=93N1WsDx3xkI8PIGtHQm769e5Sqg9ilfdoc7k8pVcyA=; b=vyT9UD0c5ZE5B6shX8UwlIjuz7 D0ofXNzQqHqN/ia+R7/ZzzvKCoQGHMeANDGS0ABCfCUl/jDpCAkdJwLyjNqL5CrT0mpDOxN1sBYcS rXzXMopWHCVJNYwFaDgj8BHMyoVmGoT03lqSmKy5zaHJ82G5hD5+EYk9NEUMNIRc6w1eSHMx9NZWv IUaFzDfVPpEKGUzSzCHW7VDZbL0zCxVK6G7xexbv9yVcGoQkc9J60Muzrl6EC0g3acfRDsyzewrW+ nrLbmbwutPROf+5XjrFWiAF2aTsm64+jHY1rmyZ6+WaOm4jy8YZdnfa8H5OG7WCMpGFENvKpDDkST EaEo6ALw==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:35180) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1rn1nm-0006pk-10; Wed, 20 Mar 2024 19:40:42 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1rn1nd-0002vx-Rw; Wed, 20 Mar 2024 19:40:33 +0000 Date: Wed, 20 Mar 2024 19:40:33 +0000 From: "Russell King (Oracle)" To: Jiangfeng Xiao Cc: arnd@arndb.de, keescook@chromium.org, haibo.li@mediatek.com, angelogioacchino.delregno@collabora.com, amergnat@baylibre.com, akpm@linux-foundation.org, dave.hansen@linux.intel.com, douzhaolei@huawei.com, gustavoars@kernel.org, jpoimboe@kernel.org, kepler.chenxin@huawei.com, kirill.shutemov@linux.intel.com, linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, nixiaoming@huawei.com, peterz@infradead.org, wangbing6@huawei.com, wangfangpeng1@huawei.com, jannh@google.com, willy@infradead.org, David.Laight@aculab.com Subject: Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case Message-ID: References: <1709516385-7778-1-git-send-email-xiaojiangfeng@huawei.com> <1710906278-23851-1-git-send-email-xiaojiangfeng@huawei.com> <84a57ca8-8963-ca24-8bd1-ddc5c33bf4da@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <84a57ca8-8963-ca24-8bd1-ddc5c33bf4da@huawei.com> X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 7C7D31C0018 X-Stat-Signature: ijhh48jegxk7k15wqu61t54oycdjsnpf X-Rspam-User: X-HE-Tag: 1710963671-724750 X-HE-Meta: U2FsdGVkX1+fMpOZ6+k30lBh7joOw9heDnNab6Xpg1BRKV92514sFF2uDVF5BiwwuA51GBq/8PSTO9kZh773DfUo4TvClFfuB5MjhifqGi75XwW4056MofR9oKhN1BOi52hy/TQWoyKH4JlfceOIkE+A+5TLqsuupWalCiTZCeAEV1FsM66GcMSbs/u9P1MxI24rqHEAhLcubPeUsSxNlBJ8i5OqPUH492bFT84uVyflmbxRNls+1AFfAfvri3L5uK9ySVnDBbWByWijS6jeOCfPuL8XaICLGc+rRTKVEnv40Qzh0LWKmJQNtifQQgd23RVU0WfP6HR+uR5wFCIjvpwX6eTn9J63XFDePj4Mj9KmgG2Luw5b2pB7BnT0u/N4dlvI0nPo7uDvd2cf0eNsWLCo3a2RCjvTTzBo4KCj6Ph05VhDnzv+FjVqqSDphoF2bb9dWbzCMfrqGHvJ3QZJKNvS76wyg8X8f+lD1o7vMU+dB6MooGsmmnM/oXaWRhHP2AthJ+bQgqhoKEXfTqzsOBvEiPf5PDBn89pYLRj9P5Jhc0i8rIdmo/CivooeocpWOV81Kuw2QuxOcw5sr9BOeRAYPXBjLxYBCcEIUTEbTwRmS16PzR19fSO1haJBcFZ+TURTPTJmIEZecPlfQqzp+48/lFwJOY/cAkDGyGDXQ6WqHMyTk5c2kaa5ukWyrMHGVEZFOnWRDvfRacxqeKjbeXnM5wvp/mvqrzlyGKr88b6YFfaDL3HG20pMLYCf89qJAVo4EsBel+XSEP0yxR6BoiFQaajDh7FZjNdj5Mna80A5UquKz+qYL+CUTIg9//q6dJCZLSxRI1fz+I2/JREabXLMOvWDUKXBJW6DlsQgIEznrMKB/opC8i86HJEBd1EFRU6bwzAjiM+5jADoqIb9uYYqSnhhV9GaDUUn0OpabKfrc0GasEr/o9tff+fPdLLiHLbbggrAu4WBX+mkydK ZfqDOpwV lxHcgTL+XqkKaZ+Q2yyhAKKi4U0yYmKweaR/WQNq8nr2EtYw1sg7AUGBiub+KGSLQhYBTG0ueK5SFZA58zwC1gK2+q4vLmMip93/FvVlg8j/7gg6RBCzN590FYwzZ3magamCbbgsJdIcWcZ6OxuKo4AgQWPkAayARgInl7O2dWIvSGY8z8g/Y+G0azI8HEOFfZc1kq5qnd2jEWltiYGvaMzDrBiqWqDzGPhNJ5ZRwu5TSxDf2PpUxct3otcvPMh71jGru22Y6qEEdU7o9qyy8osNVDBtl/HFmyzl6kW9Y7im54DZqGfb/WTVrHp5wYHiFWkVN4QFfP5P4ZbWpFoRLdR8GwK+tkPDUzYu+3ky9rVdx9yVXDoKwWRytgNWPEdG/HsQy8OTrXzGm54ReBJBNws/5L40eyKlldou5cMmzCXuF9BvtFyEykz6otfIYs6y8US9luFEtETlNFHDfOs8vuvzQfEb8VcqEgmRIoBS3Z1S7VnHcBFPgpswe6hKNELwKIPRpP9dGPscuJZY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000104, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Mar 20, 2024 at 11:30:05PM +0800, Jiangfeng Xiao wrote: > > > On 2024/3/20 16:45, Russell King (Oracle) wrote: > > On Wed, Mar 20, 2024 at 11:44:38AM +0800, Jiangfeng Xiao wrote: > >> This is an off-by-one bug which is common in unwinders, > >> due to the fact that the address on the stack points > >> to the return address rather than the call address. > >> > >> So, for example, when the last instruction of a function > >> is a function call (e.g., to a noreturn function), it can > >> cause the unwinder to incorrectly try to unwind from > >> the function after the callee. > >> > >> foo: > >> ... > >> bl bar > >> ... end of function and thus next function ... > >> > >> which results in LR pointing into the next function. > >> > >> Fixed this by subtracting 1 from frmae->pc in the call frame > >> (but not exception frames) like ORC on x86 does. > > > > The reason that I'm not accepting this patch is because the above says > > that it fixes it by subtracting 1 from the PC value, but the patch is > > *way* more complicated than that and there's no explanation why. > > > > For example, the following are unexplained: > > > > - Why do we always need ex_frame > > ``` > bar: > ... > ... end of function bar ... > > foo: > BUG(); > ... end of function foo ... > ``` > > For example, when the first instruction of function 'foo' > is a undefined instruction, after function 'foo' is executed > to trigger an exception, 'arm_get_current_stackframe' assigns > 'regs->ARM_pc' to 'frame.pc'. > > If we always decrement frame.pc by 1, unwinder will incorrectly > try to unwind from the function 'bar' before the function 'foo'. > > So we need to 'ext_frame' to distinguish this case > where we don't need to subtract 1. This just sounds wrong to me. We have two different cases: 1) we're unwinding a frame where PC points at the offending instruction. This may or may not be in the exception text. 2) we're unwinding a frame that has been created because of a branch, where the PC points at the next instruction _after_ that callsite. While we're unwinding, we will mostly hit the second type of frames, but we'll only hit the first type on the initial frame. Some exception entries will have the PC pointing at the next instruction to be executed, others will have it pointing at the offending instruction (e.g. if it needs to be retried.) So, I don't see what being in the exception/entry text really has much to do with any decision making here. I think you've found that it works for your case, but it won't _always_ work and you're just shifting the "bug" with how these traces work from one issue to a different but similar issue. > > - What is the purpose of the change in format string for the display of > > backtraces > ``` > unwind_frame(&frame); > dump_backtrace_entry(...from) //from = frame.pc > printk("...%pS\n", ...(void *)from); > ``` > %pB will do sprint_backtrace and print the symbol at (from - 1) address > %pS will do sprint_symbol_build_id and print the symbol at (from) address The quote in printk-formats.rst states: %pS versatile_init+0x0/0x110 %pB prev_fn_of_versatile_init+0x88/0x88 This is rather ambiguous on its own, since the definition of "previous function" is ambiguous. Given the offset and size stated there, it's also not obvious what pointer was passed. It would be nice if these examples actually said what the pointer passed in actually was. I had been interpreting "prev_fn_of_" to mean the caller of versatile_init() but it could also be the preceeding function in the kernel text to versatile_init() - that is where the ambiguity comes from. So, the question I now have is... if %pB prints the symbol corresponding with "from - 1", then - with the frame pointer walker, from will always be the return address found on the stack for the function we are currently in. - with the unwinder it will be whatever the unwinder computes as the LR register unless the unwind instructions place a non-zero value in PC. Is there a case where the unwinder gets this wrong? I think what would help is if you split this patch up, and addressed each part separately, describing the issue that each part is addressing giving an example that clearly explains what the patch is doing. However, please note my comments above that using the fact that we're in an exception frame doesn't actually tell you anything about whether you need to correct the PC value or not. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!