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 X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 37431C43381 for ; Mon, 1 Mar 2021 17:00:27 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 025CD64F4B for ; Mon, 1 Mar 2021 17:00:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 025CD64F4B 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=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc: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:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=rhYDD0tB3bgJT1Wd3ets+mRapGx1oVqbZpv2Mwd/9fU=; b=QaQUbifngdQNKUPpyThf7NxyQ cNHFmwnijntSwBkCkQS9TeKq+OlYBxAUGbjKNFznV8Ouy8M0p9xGHBk+Vlz55PHu4gIKWmV3vMywV UR5caE0tOCc69Mk2i1HWgT/1H/4oFMwNCTGo5PH4GGTaUzVH68POJm6Sc5Yqyot9rbYVz1Dg1WJRu CzMRkj468UsUMR78lHd60f7quUlZJXZBuGPQWKYT9SvhelyHmeiTjZC1/abXwdJ2+s6LRTYX7c+C4 aIW5dfCRUKxcPYSd7MGYpNuxVLtjYb5FCgeLWpprCQdX3qR6tJvNN7K83a5ED0if0dlSk6o5XNyPZ grh2qEeww==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lGlsn-000566-BW; Mon, 01 Mar 2021 16:58:57 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lGlsk-00054Q-AO for linux-arm-kernel@lists.infradead.org; Mon, 01 Mar 2021 16:58:55 +0000 Received: from [192.168.254.32] (unknown [47.187.194.202]) by linux.microsoft.com (Postfix) with ESMTPSA id DA81A20B57A1; Mon, 1 Mar 2021 08:58:51 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com DA81A20B57A1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1614617932; bh=5iLiT4zCUvpdCRGwIN7OylfYUPKmTAZ9Z3KeVffXDOc=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=dS1CAaa3yJ0UPOnsU90ZhqIFUaGnM5whBFQxd4ni8FzqozwSNdhHqCBBMfo4UpdRZ yNWiIqBtGNJ7FyPngJzbZcDNLT1EqeU3tLQjuKaN0CVz+YPzSYOuTqxKLDikiqRP09 sQglsDNjAYMPV/CAe9LfgyevWmJNRu5VzfExbUXM= Subject: Re: [RFC PATCH v1 1/1] arm64: Unwinder enhancements for reliable stack trace To: Mark Rutland References: <20210223181243.6776-1-madvenka@linux.microsoft.com> <20210223181243.6776-2-madvenka@linux.microsoft.com> <20210224121716.GE50741@C02TD0UTHF1T.local> <4a96b31d-0d16-1f12-fa64-5fdae64cd2d1@linux.microsoft.com> <20210225115825.GB37015@C02TD0UTHF1T.local> From: "Madhavan T. Venkataraman" Message-ID: <0227d386-c392-eb5a-3f52-621a637e46a8@linux.microsoft.com> Date: Mon, 1 Mar 2021 10:58:51 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210225115825.GB37015@C02TD0UTHF1T.local> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210301_115854_605616_49FD2908 X-CRM114-Status: GOOD ( 30.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jthierry@redhat.com, linux-kernel@vger.kernel.org, broonie@kernel.org, jpoimboe@redhat.com, live-patching@vger.kernel.org, linux-arm-kernel@lists.infradead.org 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 On 2/25/21 5:58 AM, Mark Rutland wrote: > On Wed, Feb 24, 2021 at 01:34:09PM -0600, Madhavan T. Venkataraman wrote: >> On 2/24/21 6:17 AM, Mark Rutland wrote: >>> On Tue, Feb 23, 2021 at 12:12:43PM -0600, madvenka@linux.microsoft.com wrote: >>>> From: "Madhavan T. Venkataraman" >>>> Termination >>>> =========== >>>> >>>> Currently, the unwinder terminates when both the FP (frame pointer) >>>> and the PC (return address) of a frame are 0. But a frame could get >>>> corrupted and zeroed. There needs to be a better check. >>>> >>>> The following special terminating frame and function have been >>>> defined for this purpose: >>>> >>>> const u64 arm64_last_frame[2] __attribute__ ((aligned (16))); >>>> >>>> void arm64_last_func(void) >>>> { >>>> } >>>> >>>> So, set the FP to arm64_last_frame and the PC to arm64_last_func in >>>> the bottom most frame. >>> >>> My expectation was that we'd do this per-task, creating an empty frame >>> record (i.e. with fp=NULL and lr=NULL) on the task's stack at the >>> instant it was created, and chaining this into x29. That way the address >>> is known (since it can be derived from the task), and the frame will >>> also implicitly check that the callchain terminates on the task stack >>> without loops. That also means that we can use it to detect the entry >>> code going wrong (e.g. if the SP gets corrupted), since in that case the >>> entry code would place the record at a different location. >> >> That is exactly what this is doing. arm64_last_frame[] is a marker frame >> that contains fp=0 and pc=0. > > Almost! What I meant was that rather that each task should have its own > final/marker frame record on its task task rather than sharing a common > global variable. > > That way a check for that frame record implicitly checks that a task > started at the expected location *on that stack*, which catches > additional stack corruption cases (e.g. if we left data on the stack > prior to an EL0 entry). > Ok. I will think about this. > [...] > >>> ... I reckon once we've moved the last of the exception triage out to C >>> this will be relatively simple, since all of the exception handlers will >>> look like: >>> >>> | SYM_CODE_START_LOCAL(elX_exception) >>> | kernel_entry X >>> | mov x0, sp >>> | bl elX_exception_handler >>> | kernel_exit X >>> | SYM_CODE_END(elX_exception) >>> >>> ... and so we just need to identify the set of elX_exception functions >>> (which we'll never expect to take exceptions from directly). We could be >>> strict and reject unwinding into arbitrary bits of the entry code (e.g. >>> if we took an unexpected exception), and only permit unwinding to the >>> BL. >>> >>>> It can do this if the FP and PC are also recorded elsewhere in the >>>> pt_regs for comparison. Currently, the FP is also stored in >>>> regs->regs[29]. The PC is stored in regs->pc. However, regs->pc can >>>> be changed by lower level functions. >>>> >>>> Create a new field, pt_regs->orig_pc, and record the return address >>>> PC there. With this, the unwinder can validate the exception frame >>>> and set a flag so that the caller of the unwinder can know when >>>> an exception frame is encountered. >>> >>> I don't understand the case you're trying to solve here. When is >>> regs->pc changed in a way that's problematic? >>> >> >> For instance, I used a test driver in which the driver calls a function >> pointer which is NULL. The low level fault handler sends a signal to the >> task. Looks like it changes regs->pc for this. > > I'm struggling to follow what you mean here. > > If the kernel branches to NULL, the CPU should raise an instruction > abort from the current EL, and our handling of that should terminate the > thread via die_kernel_fault(), without returning to the faulting > context, and without altering the PC in the faulting context. > > Signals are delivered to userspace and alter the userspace PC, not a > kernel PC, so this doesn't seem relevant. Do you mean an exception fixup > handler rather than a signal? > >> When I dump the stack from the low level handler, the comparison with >> regs->pc does not work. But comparison with regs->orig_pc works. > > As above, I'm struggling with this; could you share a concrete example? > Actually, my bad. I needed the orig_pc because of something that my test driver did. And, it slipped my mind entirely. Thanks for pointing it out. I will fix this in my resend. Thanks again for your comments. I am currently studying probing/tracing. As soon as I have a solution for that, I will send out the next version. I look forward to the in-depth review. Thanks, Madhavan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel