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 476EAC433EF for ; Sun, 8 May 2022 17:46:12 +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-Type: Content-Transfer-Encoding:Reply-To:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AKS6nONdmtWD/b27xdNfHEssgjjhGeJJZcNCffw8lcQ=; b=w20HOnIIjQzIQy ZQ5Kso3+UhFizCk2+UWmj4os3GmVMfnjUWOy16WkLCbsBGdA6uMZoGur6XwhIr6LbHcvt3n30+NVA pWPMYsAYlytpOqb4Ll6d/hrOuI7H9QX0rJvwAAKnsP2PRPwx+Mei472Ufsa+qxcJ0tyIZD2jJFNjh Yf9kSlrgkzxs7XK0FjIa6dsn6QMgPQfbS1FiSzYih221gADSJ5x1Jhl0FMnB4W+uNF49VnnFGOV/r 8wQhKqvEK7y79DwGTw1QmINelfSRBHbdOJU7E9dUN96hkOlg07kaukuWt1AUWXQntveGKhE7nvHD2 +bRonW+nxCGrqNTnS4MQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nnkxt-00Am9p-DY; Sun, 08 May 2022 17:45:05 +0000 Received: from mail-ej1-f53.google.com ([209.85.218.53]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nnkxq-00Am8e-0w for linux-arm-kernel@lists.infradead.org; Sun, 08 May 2022 17:45:03 +0000 Received: by mail-ej1-f53.google.com with SMTP id z2so21319661ejj.3 for ; Sun, 08 May 2022 10:45:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:reply-to :subject:content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=k3E+pHwvsluqh4RQWxDEEF6Q4LCI2lL0+EMS4NMvu6M=; b=SnmCgMe21mEQR/fFgNwUMICCPN/O1jVrRxcSUix4uJFNb4iKamVts9E4wXBuRTcMz/ pE2o5EoNfgWuWftkKIBWHVIg0gzsuwxH+cjCFnhGYbF5mnTWEqJvIRAoU78Gb6xJK32Z R9395KO0CYDfwck/fc9wWXvZj28BMURNKQXtyvsbHjB4QYPGwDyHVi2qqIFprCdoNcPV /4KV2JozTGF4Y0epsj/SfX6H4PweNLPFk6uG51VDvLpFn1CIyyYBv6HwWL/XBNPm3+7m lYBoWynqyJS5RrhN1EjkuH+9XpOOebOYKKwbSgEbk2WQkD5GMh4yqsrcm8ULDtuK9NdC Y+ZQ== X-Gm-Message-State: AOAM5321rDXlRJ1udkqBVw8CBhGyZGCTzvWFOMz2qvF3xSi5//vqA0z6 NRSYkjW0ZB3mLzX4ljZwzc4= X-Google-Smtp-Source: ABdhPJwMBsbinqBwxW5+LE53BGDzuuqoEkSLWLSeIaJVJXv4ng7tNLsopng36LHkr0SoO/4JbQ1hTQ== X-Received: by 2002:a17:907:d01:b0:6f4:d873:d7a0 with SMTP id gn1-20020a1709070d0100b006f4d873d7a0mr11633638ejc.717.1652031900107; Sun, 08 May 2022 10:45:00 -0700 (PDT) Received: from [10.9.0.34] ([46.166.128.205]) by smtp.gmail.com with ESMTPSA id wi22-20020a170906fd5600b006f3ef214de0sm4237170ejb.70.2022.05.08.10.44.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 08 May 2022 10:44:59 -0700 (PDT) Message-ID: <51cea283-3cc7-2361-413c-d1bd8ac845bb@linux.com> Date: Sun, 8 May 2022 20:44:56 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v2 02/13] stackleak: move skip_erasing() check earlier Content-Language: en-US To: Mark Rutland , linux-arm-kernel@lists.infradead.org Cc: akpm@linux-foundation.org, catalin.marinas@arm.com, keescook@chromium.org, linux-kernel@vger.kernel.org, luto@kernel.org, will@kernel.org References: <20220427173128.2603085-1-mark.rutland@arm.com> <20220427173128.2603085-3-mark.rutland@arm.com> From: Alexander Popov In-Reply-To: <20220427173128.2603085-3-mark.rutland@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220508_104502_140564_E55DBAAC X-CRM114-Status: GOOD ( 27.84 ) 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: , Reply-To: alex.popov@linux.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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. > > Before this patch, on x86-64 w/ GCC 11.1.0 the start of the function is: > > : > 65 48 8b 04 25 00 00 mov %gs:0x0,%rax > 00 00 > 48 8b 48 20 mov 0x20(%rax),%rcx > 48 8b 80 98 0a 00 00 mov 0xa98(%rax),%rax > 66 90 xchg %ax,%ax <------------ static branch > 48 89 c2 mov %rax,%rdx > 48 29 ca sub %rcx,%rdx > 48 81 fa ff 3f 00 00 cmp $0x3fff,%rdx > > After this patch, on x86-64 w/ GCC 11.1.0 the start of the function is: > > : > 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) <--- static branch > 65 48 8b 04 25 00 00 mov %gs:0x0,%rax > 00 00 > 48 8b 48 20 mov 0x20(%rax),%rcx > 48 8b 80 98 0a 00 00 mov 0xa98(%rax),%rax > 48 89 c2 mov %rax,%rdx > 48 29 ca sub %rcx,%rdx > 48 81 fa ff 3f 00 00 cmp $0x3fff,%rdx > > Before this patch, on arm64 w/ GCC 11.1.0 the start of the function is: > > : > d503245f bti c > d5384100 mrs x0, sp_el0 > f9401003 ldr x3, [x0, #32] > f9451000 ldr x0, [x0, #2592] > d503201f nop <------------------------------- static branch > d503233f paciasp > cb030002 sub x2, x0, x3 > d287ffe1 mov x1, #0x3fff > eb01005f cmp x2, x1 > > After this patch, on arm64 w/ GCC 11.1.0 the start of the function is: > > : > d503245f bti c > d503201f nop <------------------------------- static branch > d503233f paciasp > d5384100 mrs x0, sp_el0 > f9401003 ldr x3, [x0, #32] > d287ffe1 mov x1, #0x3fff > f9451000 ldr x0, [x0, #2592] > cb030002 sub x2, x0, x3 > eb01005f cmp x2, x1 > > While this may not be a huge win on its own, moving the static branch > will permit further optimization of the body of the function in > subsequent patches. > > Signed-off-by: Mark Rutland > Cc: Alexander Popov > Cc: Andrew Morton > Cc: Andy Lutomirski > Cc: Kees Cook > --- > kernel/stackleak.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > 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? > { > /* 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