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=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 55E05C433E0 for ; Mon, 1 Feb 2021 16:03:59 +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 0B2ED64D7F for ; Mon, 1 Feb 2021 16:03:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0B2ED64D7F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.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:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=J+Vm6dQMYfZc2rbX4EdAgR/7R6tLDCBs7CEEpk+I41M=; b=2QFDDvWh3VO2rxaEhHMvV30XQ 3P4+dKMt3UHWJA93iGR39wDXrQnvHT9xk7N+XE1PowP07KEvapqvswMHh0Fii3cYjWBr/VaJXqaLH 8AijP/Qh5vrmX+lG8+OzsDnNMp32Jgt6HQulR2eAG8CzbT9XD2pjQ+MUtIljHvBy3mPI1LfyMmMHN 9P23Qwe4uDdTr22FJ1eynDjOUwCFA28DEjUR98xSkkCr5Q1NYIcP8IW1BKnQCPMD9SMX82x6Lm64a GWvmjGBG/OUHvvSUgRvEgYsZlu2Z4h5OupIKdQ6hDBglQlF1Qr10JejuDS10iG1a8mJHk0VlAObbh ZsODcAFvg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6bes-0007RT-8Y; Mon, 01 Feb 2021 16:02:34 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6bep-0007R4-CG for linux-arm-kernel@lists.infradead.org; Mon, 01 Feb 2021 16:02:32 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3598F1042; Mon, 1 Feb 2021 08:02:29 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.41.104]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D3AB73F718; Mon, 1 Feb 2021 08:02:27 -0800 (PST) Date: Mon, 1 Feb 2021 16:02:25 +0000 From: Mark Rutland To: "Madhavan T. Venkataraman" Subject: Re: [RFC PATCH 0/3] arm64: Implement reliable stack trace Message-ID: <20210201160225.GD66060@C02TD0UTHF1T.local> References: <20201012172605.10715-1-broonie@kernel.org> <13095563-ff6d-b806-1bf3-efde4383456e@linux.microsoft.com> <20210128142250.GC4537@sirena.org.uk> <20210128152649.6zin3hzim3etbv2p@treble> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210201_110231_538508_E29B189C X-CRM114-Status: GOOD ( 36.08 ) 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: Julien Thierry , Catalin Marinas , Mark Brown , Josh Poimboeuf , Miroslav Benes , Will Deacon , 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 Hi Madhavan, On Mon, Feb 01, 2021 at 09:21:43AM -0600, Madhavan T. Venkataraman wrote: > On 1/28/21 9:26 AM, Josh Poimboeuf wrote: > >> If we're trusting the compiler we can probably just do that without any > >> explicit support from the compiler - it should be doing the standard > >> stuff unless we explicitly ask it to and if it isn't then it might be a > >> result of a mismatch in assumptions rather than a deliberate decision to > >> do something non-standard. My understanding with objtool is that a big > >> part of the idea is to provide a static check that the binary we end up > >> with matches the assumptions that we are making so the fact that it's a > >> separate implementation is important. > > For C code, even if we trusted the compiler (which we don't), we still > > have inline asm which the compiler doesn't have any visibility to, which > > is more than capable of messing up frame pointers (we had several cases > > of this in x86). > > > > Getting the assembler to annotate which functions are FP could be > > interesting but: > > > > a) good luck getting the assembler folks to do that; they tend to be > > insistent on being ignorant of code semantics; > > > > b) assembly often resembles spaghetti and the concept of a function is > > quite fluid; in fact many functions aren't annotated as such. > > OK. Before this whole discussion, I did not know that the compiler cannot be trusted. I think "the compiler cannot be trusted" is overly strong. We want to *verify* that the compiler is doing what we expect, which might be more than what it guarantees to do (and those expectations can change over time), but it doesn't mean we should try to avoid the compiler wherever possible. For assembly I expect we'll need to do /some/ manual annotation (e.g. for trampolines). > So, it looks like objtool is definitely needed. However, I believe we can minimize > the work objtool does by using a shadow stack. > > I read Mark Brown's response to my shadow stack email. I agree with him. The shadow > stack looks promising. > > So, here is my suggestion for the shadow stack. This is just to start the discussion > on the shadow stack. Regarding unwinding, shadow stacks have the same problems as frame records (considering exceptions/interrupts) in that the primary problem is knowing *when* they are updated, and knowing *when* the LR is valid or invalid or duplicated (in a frame record or shadow stack entry). Given that, I think that assuming we must use a shadow stack for reliable unwinding would be jumping the gun. > Prolog and epilog for C functions > ================================= > > Some shadow stack prolog and epilog are needed. Let us add a new option to the compiler > to generate extra no-ops at the beginning of a function for the prolog and just before > return for the epilog so some other entity such as objtool can add its own prolog and > epilog. This is so we don't have to trust the compiler and can maintain our own prolog > and epilog. Why wouldn't we ask the compiler to to this, and just check this in the tooling? ... and if we can do that, why not just check the frame pointer manipulation? Note that functions can have multiple return paths, and there might not be one epilogue. Also, today some functions can have special-cased prologues for early checks, e.g. | function: | CBNZ X0, _func | RET | STP X29, X30, [SP, #-FRAME_SIZE] | MOV X29, X30 | ... | LDP X29, X30, [SP], #FRAME_SIZE | RET ... and forcing additional work in those could be detrimental. > Objtool will check for the no-ops. If they are present, it will replace the no-ops with > the shadow stack prolog and epilog. It can also check the frame pointer prolog and > epilog. I suspect this will interact poorly with patchable-function-entry, which prefixes each instrumentable function with some NOPs. > Then, it will set a flag in the symbol table entry of the function to indicate that > the function has a proper prolog and epilog. I think this boils down to having a prologue and epilogue check, which seems sane. > Prolog and epilog for assembly functions > ======================================== > > The no-ops and frame pointer prolog and epilog can be added to assembly functions manually. > Objtool will process them as above. > > Decoding > ======== > > To do all this, objtool has to decode only the following instructions. > > - no-op > - return instruction > - store register pair in frame pointer prolog > - load register pair in frame pointer epilog > > This simplifies the objtool part a lot. AFAIK, all instructions in ARM64 are > 32 bits wide. So, objtool does not have to decode an instruction to know its > length. > > Objtool has to scan a function for the return instruction to know the location(s) > of the epilog. That wouldn't be robust if you consider things like: | func: | STP x29, x30, [SP, #-FRAME_SIZE]! | MOV X29, SP | B __fake_ret | LDP X29, X30, [SP], #FRAME_SIZE | RET | __fake_ret: | BL x30 ... which is the sort of thing we want objtool to catch. [...] > Unwinder logic > ============== > > The unwinder will walk the stack using frame pointers like it does > currently. As it unwinds the regular stack, it will also unwind the > shadow stack: > > However, at each step, it needs to perform some additional checks: > > symbol = lookup symbol table entry for pc > if (!symbol) > return -EINVAL; > > if (symbol does not have proper prolog and epilog) > return -EINVAL; I think at this point, we haven't gained anything from using a shadow stack, and I'd much rather we used objtool to gather any metadata needed to make unwinding reliable without mandating a shadow stack. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel