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 AC282C433F5 for ; Tue, 10 May 2022 11:41:36 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=py0788hPEQ8NNVp3aFtXnKEib0Qqwx4dcWLDABSEObg=; b=kdXGkzFB7nFOJz Ecz9mSCP6ZTZkwIRsd70gOGTNncHTIrYmRgqYm0f/pmYN0/6XryTFwO19IWF6Bd7xhL0ksZzGq8ok WYkygyvZovi5/zuXh1fJ+VYEsRCHanQDhimVwauEVbXQQ9abpM2XMOTiF1oJVggy5azHLL8tumiA0 Ff5AItmR4M/zUiowS2TjXkjW3j/C8VRUgIV1EQr54PFxWefsWE7ic9/snmuF7KIyFs0SaRwi8EMC/ hM0N5qHrZBB45KVBtOvKDSFRpRSHG65Ic9Akch+kAPLeZhiHLsylI49VlLHBZA6r/ZpLkzZy62PyV otwdHlWVrQ+1xGFnh1gA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1noOEA-001eAi-4O; Tue, 10 May 2022 11:40:30 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1noOE4-001e7v-Iv for linux-arm-kernel@lists.infradead.org; Tue, 10 May 2022 11:40:26 +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 AB23A11FB; Tue, 10 May 2022 04:40:22 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.1.67]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 37DCC3F66F; Tue, 10 May 2022 04:40:21 -0700 (PDT) Date: Tue, 10 May 2022 12:40:17 +0100 From: Mark Rutland To: Alexander Popov Cc: linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, will@kernel.org Subject: Re: [PATCH v2 02/13] stackleak: move skip_erasing() check earlier Message-ID: References: <20220427173128.2603085-1-mark.rutland@arm.com> <20220427173128.2603085-3-mark.rutland@arm.com> <51cea283-3cc7-2361-413c-d1bd8ac845bb@linux.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <51cea283-3cc7-2361-413c-d1bd8ac845bb@linux.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220510_044024_737981_055A015F X-CRM114-Status: GOOD ( 28.74 ) 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 On Sun, May 08, 2022 at 08:44:56PM +0300, Alexander Popov wrote: > On 27.04.2022 20:31, Mark Rutland wrote: > > In stackleak_erase() we check skip_erasing() after accessing some fields > > from current. As generating the address of current uses asm which > > hazards with the static branch asm, this work is always performed, even > > when the static branch is patched to jump to the return a the end of the > > function. > > Nice find! > > > This patch avoids this redundant work by moving the skip_erasing() check > > earlier. > > > > To avoid complicating initialization within stackleak_erase(), the body > > of the function is split out into a __stackleak_erase() helper, with the > > check left in a wrapper function. The __stackleak_erase() helper is > > marked __always_inline to ensure that this is inlined into > > stackleak_erase() and not instrumented. [...] > > diff --git a/kernel/stackleak.c b/kernel/stackleak.c > > index ddb5a7f48d69e..753eab797a04d 100644 > > --- a/kernel/stackleak.c > > +++ b/kernel/stackleak.c > > @@ -70,7 +70,7 @@ late_initcall(stackleak_sysctls_init); > > #define skip_erasing() false > > #endif /* CONFIG_STACKLEAK_RUNTIME_DISABLE */ > > -asmlinkage void noinstr stackleak_erase(void) > > +static __always_inline void __stackleak_erase(void) > > Are you sure that __stackleak_erase() doesn't need asmlinkage and noinstr as well? I am certain it needs neither. It's static and never called from asm, so it doesn't need `asmlinkage`. It's marked `__always_inline`, so it will always be inlined into its caller (or if the compiler cannot inline it, will result in a compiler error). That's important to get good codegen (especially with the on/off stack variants later in the series), and when inlined into its caller the compiler will treat it as part of its caller for code generation, so the caller's `noinstr` takes effect. Thanks, Mark. > > > { > > /* It would be nice not to have 'kstack_ptr' and 'boundary' on stack */ > > unsigned long kstack_ptr = current->lowest_stack; > > @@ -78,9 +78,6 @@ asmlinkage void noinstr stackleak_erase(void) > > unsigned int poison_count = 0; > > const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long); > > - if (skip_erasing()) > > - return; > > - > > /* Check that 'lowest_stack' value is sane */ > > if (unlikely(kstack_ptr - boundary >= THREAD_SIZE)) > > kstack_ptr = boundary; > > @@ -125,6 +122,14 @@ asmlinkage void noinstr stackleak_erase(void) > > current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64; > > } > > +asmlinkage void noinstr stackleak_erase(void) > > +{ > > + if (skip_erasing()) > > + return; > > + > > + __stackleak_erase(); > > +} > > + > > void __used __no_caller_saved_registers noinstr stackleak_track_stack(void) > > { > > unsigned long sp = current_stack_pointer; > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel