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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BA12C6FD1F for ; Wed, 8 Mar 2023 10:28:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B5A6E6B0072; Wed, 8 Mar 2023 05:28:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B0A136B0074; Wed, 8 Mar 2023 05:28:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F8D26B0075; Wed, 8 Mar 2023 05:28:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 91A996B0072 for ; Wed, 8 Mar 2023 05:28:24 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3C906A34F7 for ; Wed, 8 Mar 2023 10:28:24 +0000 (UTC) X-FDA: 80545356528.07.AEF7D84 Received: from mail.skyhub.de (mail.skyhub.de [5.9.137.197]) by imf18.hostedemail.com (Postfix) with ESMTP id 9CDF11C0011 for ; Wed, 8 Mar 2023 10:28:14 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=temperror ("DNS error when getting key") header.d=alien8.de header.s=dkim header.b=qKM1WjO9; spf=temperror (imf18.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) Received: from zn.tnic (p5de8e9fe.dip0.t-ipconnect.de [93.232.233.254]) (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 A29E81EC0104; Wed, 8 Mar 2023 11:28:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1678271289; 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=QUSal8yLRZqvOQ34incj8gT6TWJsFqJ8/OvBTCTsShk=; b=qKM1WjO9F9dXO6k5BZ17YhP7pKauIgrnWTcEAhizPLCqjIUDtUBoqz9Vv7RlAmfM7ENNcX Np7CELn8CU+Qrg3p56Rww0dIFPR7Hx8s81XMMztPWSMC3ak4lXSGrlcQPEiXxVGph6KZkV fp3eA1tDCcaGYqrcE+cPk4ynDNL4gOU= Date: Wed, 8 Mar 2023 11:27:56 +0100 From: Borislav Petkov To: Rick Edgecombe 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 , Weijiang Yang , "Kirill A . Shutemov" , John Allen , kcc@google.com, eranian@google.com, rppt@kernel.org, jamorris@linux.microsoft.com, dethoma@microsoft.com, akpm@linux-foundation.org, Andrew.Cooper3@citrix.com, christina.schimpe@intel.com, david@redhat.com, debug@rivosinc.com Subject: Re: [PATCH v7 28/41] x86: Introduce userspace API for shadow stack Message-ID: References: <20230227222957.24501-1-rick.p.edgecombe@intel.com> <20230227222957.24501-29-rick.p.edgecombe@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20230227222957.24501-29-rick.p.edgecombe@intel.com> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 9CDF11C0011 X-Stat-Signature: 754j8w8o3ed3qjbsc4cyhztc7uhbfx5u X-HE-Tag: 1678271294-23150 X-HE-Meta: U2FsdGVkX1/d4+hfG90xqCbrk38l/jO4HnTMslCHVPClAR4qEmPq26UFGpz7RRpja+t4znfhSLvUL3jhVR+OBaDqEByKxokfWNuUOz7zvG09fhsi56mkKEnGxli69Wt9VxEGF97GdvCjX5QHG8c7RMDyd70CqwglaC9y/PbFC8EBU+i7krBcapxZJRAeG4SvxvWOtkqe4fdu/UQOT8FRMbYPduwjQDgvtgeUAXt8WpHg9gU2qE7zpRMfHAIk68z39/FzF/CG0pVtasW0wxRfTLYfaAz2wrbB4ZFryvDLQ4qFe0Szvdhk8/f41fR/eWl6rW3mh/enPMXzxJ5XwDT0P06vxk0WzVz9z9pKKWacDsDYSnrwU81PNqUktezXIRp/rwNwQ23BJaOdFFzmgi288hf7ts+FwRi1J5aHHMiN+Ol0q6HUvQcoi8t2c/iAdifLhmEPg7FVgZvk0d8/kSLHCluWytLvkyRO1RIOOVyK8FbcfmQH93oBD02gNFUeIY7MhdKiC8xPayGESY33lLfw18lKJnZuj09Dw0KdpohBKYh1rtTUl+JX68Yi5M12SZxp5HmH20MKKZ2zORJehhD4zglOeNKGp0ON5icNczUyM80i/GeUKMdDazPkBfaK+i2OMP7pv8c+NkjvBadYtk2zAGt5zMpZS4GO1WghsB6CZbJLNCqmFQ7BgSU1uSO+DfkKxjxJ2LBhwMg6BIjUoHxaZ1AxoZHQ2JRKzQhe0s1I9f99R8l7VWiQgB5GIJzFYeXNGXJ7kg4o/Glhf4tm0bpeLsyxrWXAMqBVelK0e7hMGXC3V5UJ6Jwkh1FNDBOElBkZAW7B9oV+SkKNS7WxTAQ5csRGl+z+BpdM/AO4Q1u2e+sK2dvUYDhrajXUPdt6NxPZGBChlU4jbk7vZWq9e8qnia7xP4X5+G0nc/z9p7YfNuKfAAZyG5g5qqeSEJGQzLOLAz7YBfGj2ub20qtYzdV pAM8St10 1FMKKFQSZ0gpOMTRjRCfUUFauvO4lAAdvlRL0sf4UFLIEjaJeq+CCQInuEQZfi15sbnycYsO6bdSb6RAcIRfu4wj3FxWCGjayVZZb92wYodh1Rp4O4mwsNeLp/UTOLrcCjlSIfP7IPLOjWDiFva6Vp5J6FxWrtMRVNc8PIofrmH6czzU9Q4WcWkbhA5NNv5O2qiKkKT67SEwLra5yV0XPPNlpIiMe/tshfIWfkoaDCl9as1hbHgf/jmXO1fpHwV7ZxeLowvlLWsVXrul3mBJWtwxY7e9gR+MkMfBK/wd0zhG1mG0OFi9vEwuYdn1B5blHWgsgaE+EAC+pmDWJ+7X9JisyxayqqQ3hKhdr1KTkC9IdM3B2uS3r8cwWSwAF/MFYstheQICqmhR4jIlwpi1NwBwSyg29N7UqBhP+1PiBj1pMApfgEad6wCdY5Og9kVB0M3ClO25k/j7cBxnVqSkPUF80042Wsz0EbzYdm6TAGZA1ZXAcUNnaTeXnJZXHaWL7gJi8 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, Feb 27, 2023 at 02:29:44PM -0800, Rick Edgecombe wrote: > From: "Kirill A. Shutemov" > > Add three new arch_prctl() handles: > > - ARCH_SHSTK_ENABLE/DISABLE enables or disables the specified > feature. Returns 0 on success or an error. "... or a negative value on error." > - ARCH_SHSTK_LOCK prevents future disabling or enabling of the > specified feature. Returns 0 on success or an error ditto. What is the use case of the feature locking? I'm under the simple assumption that once shstk is enabled for an app, it remains so. I guess my question is rather, what's the use case for enabling shadow stack and then disabling it later for an app...? > The features are handled per-thread and inherited over fork(2)/clone(2), > but reset on exec(). > > This is preparation patch. It does not implement any features. That belongs under the "---" line I guess. > Tested-by: Pengfei Xu > Tested-by: John Allen > Tested-by: Kees Cook > Acked-by: Mike Rapoport (IBM) > Reviewed-by: Kees Cook > Signed-off-by: Kirill A. Shutemov > [tweaked with feedback from tglx] > Co-developed-by: Rick Edgecombe > Signed-off-by: Rick Edgecombe > > --- > v4: > - Remove references to CET and replace with shadow stack (Peterz) > > v3: > - Move shstk.c Makefile changes earlier (Kees) > - Add #ifdef around features_locked and features (Kees) > - Encapsulate features reset earlier in reset_thread_features() so > features and features_locked are not referenced in code that would be > compiled !CONFIG_X86_USER_SHADOW_STACK. (Kees) > - Fix typo in commit log (Kees) > - Switch arch_prctl() numbers to avoid conflict with LAM > > v2: > - Only allow one enable/disable per call (tglx) > - Return error code like a normal arch_prctl() (Alexander Potapenko) > - Make CET only (tglx) > --- > arch/x86/include/asm/processor.h | 6 +++++ > arch/x86/include/asm/shstk.h | 21 +++++++++++++++ > arch/x86/include/uapi/asm/prctl.h | 6 +++++ > arch/x86/kernel/Makefile | 2 ++ > arch/x86/kernel/process_64.c | 7 ++++- > arch/x86/kernel/shstk.c | 44 +++++++++++++++++++++++++++++++ > 6 files changed, 85 insertions(+), 1 deletion(-) > create mode 100644 arch/x86/include/asm/shstk.h > create mode 100644 arch/x86/kernel/shstk.c ... > +long shstk_prctl(struct task_struct *task, int option, unsigned long features) > +{ > + if (option == ARCH_SHSTK_LOCK) { > + task->thread.features_locked |= features; > + return 0; > + } > + > + /* Don't allow via ptrace */ > + if (task != current) > + return -EINVAL; > + > + /* Do not allow to change locked features */ > + if (features & task->thread.features_locked) > + return -EPERM; > + > + /* Only support enabling/disabling one feature at a time. */ > + if (hweight_long(features) > 1) > + return -EINVAL; > + > + if (option == ARCH_SHSTK_DISABLE) { > + return -EINVAL; > + } {} braces left over from some previous version. Can go now. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette