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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 691D5C433B4 for ; Tue, 18 May 2021 05:56:27 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 03E6B61377 for ; Tue, 18 May 2021 05:56:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 03E6B61377 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=alien8.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1A8AC6B00B5; Tue, 18 May 2021 01:56:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 166778E000D; Tue, 18 May 2021 01:56:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F3C738E000C; Tue, 18 May 2021 01:56:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0202.hostedemail.com [216.40.44.202]) by kanga.kvack.org (Postfix) with ESMTP id C22906B00B5 for ; Tue, 18 May 2021 01:56:25 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 4ED566129 for ; Tue, 18 May 2021 05:56:25 +0000 (UTC) X-FDA: 78153291930.02.604DA6F Received: from mail.skyhub.de (mail.skyhub.de [5.9.137.197]) by imf07.hostedemail.com (Postfix) with ESMTP id 1DD2FA000382 for ; Tue, 18 May 2021 05:56:14 +0000 (UTC) Received: from zn.tnic (p200300ec2f0ae2005a49629880ac02d2.dip0.t-ipconnect.de [IPv6:2003:ec:2f0a:e200:5a49:6298:80ac:2d2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id A18381EC04BF; Tue, 18 May 2021 07:56:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1621317374; h=from:from: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; bh=CoUZNshFKwxzJxvmNesTEWJlLKO0exgHkOr0ngGYwGI=; b=W6eJZYjN6OnGJsyDztzN/sdVdUAbQx1dIeHKqupFUaPUwJGH0Cv+k6IWqpHcsdMOFLs4Om 2yzHcQIIT7BswXG35L+1FYQ2Q3QHSgvXmQrXO2fond/n0k1FEGWEXFfiA7QWi3O/ZYUVj+ fZFj44LzauzVdhV4ExYc6fy4mBxiFRs= Date: Tue, 18 May 2021 07:56:09 +0200 From: Borislav Petkov To: "Yu, Yu-cheng" Cc: x86@kernel.org, "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, Arnd Bergmann , Andy Lutomirski , Balbir Singh , Cyrill Gorcunov , Dave Hansen , Eugene Syromiatnikov , Florian Weimer , "H.J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , Randy Dunlap , "Ravi V. Shankar" , Vedvyas Shanbhogue , Dave Martin , Weijiang Yang , Pengfei Xu , Haitao Huang Subject: Re: [PATCH v26 24/30] x86/cet/shstk: Introduce shadow stack token setup/verify routines Message-ID: References: <20210427204315.24153-1-yu-cheng.yu@intel.com> <20210427204315.24153-25-yu-cheng.yu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 1DD2FA000382 Authentication-Results: imf07.hostedemail.com; dkim=temperror ("DNS error when getting key") header.d=alien8.de header.s=dkim header.b=W6eJZYjN; spf=temperror (imf07.hostedemail.com: error in processing during lookup of bp@alien8.de: DNS error) smtp.mailfrom=bp@alien8.de; dmarc=temperror reason="query timed out" header.from=alien8.de (policy=temperror) X-Rspamd-Server: rspam04 X-Stat-Signature: rojxxnd9pz96yz51uybof4yg6zymt3mu X-HE-Tag: 1621317374-385089 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, May 17, 2021 at 01:55:01PM -0700, Yu, Yu-cheng wrote: > If 32-bit apps are not supported, there should be no need of 32-bit shadow > stack write, otherwise there is a bug. Aha, just a precaution. Then you can reduce the ifdeffery a bit (ontop of yours): --- diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h index f962da1fe9b5..5b48c91fa8d4 100644 --- a/arch/x86/include/asm/special_insns.h +++ b/arch/x86/include/asm/special_insns.h @@ -235,9 +235,14 @@ static inline void clwb(volatile void *__p) } #ifdef CONFIG_X86_SHADOW_STACK -#if defined(CONFIG_IA32_EMULATION) || defined(CONFIG_X86_X32) static inline int write_user_shstk_32(u32 __user *addr, u32 val) { + if (WARN_ONCE(!IS_ENABLED(CONFIG_IA32_EMULATION) && + !IS_ENABLED(CONFIG_X86_X32), + "%s used but not supported.\n", __func__)) { + return -EFAULT; + } + asm_volatile_goto("1: wrussd %[val], (%[addr])\n" _ASM_EXTABLE(1b, %l[fail]) :: [addr] "r" (addr), [val] "r" (val) @@ -246,13 +251,6 @@ static inline int write_user_shstk_32(u32 __user *addr, u32 val) fail: return -EFAULT; } -#else -static inline int write_user_shstk_32(u32 __user *addr, u32 val) -{ - WARN_ONCE(1, "%s used but not supported.\n", __func__); - return -EFAULT; -} -#endif static inline int write_user_shstk_64(u64 __user *addr, u64 val) { > These are static functions. I thought that would make the static scope > clear. I can remove "_". No, "_" or "__" prefixed functions are generally supposed to denote internal interfaces which should not be used by other kernel facilities. In that case you have the external api and the lower level helpers _, __, etc. They can be static but not necessarily. This is not the case here so you can simply drop the "_" prefixes. > If the busy bit is set, it is only for SAVEPREVSSP, and invalid as a > normal restore token. Sure but the busy bit is independent from the mode. > With the lower two bits masked out, the restore token must point > directly above itself. That I know - I'm just questioning the design. It should be addr = ALIGN_DOWN(ssp, 8); Plain and simple. Not this silly pushing and popping of stuff. But it is too late now anyway and it's not like hw people talk to software people who get to implement their shit. > Ok, then, we don't use #define's. I will put in comments about what it > is doing, and fix the rest. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette