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 0E0A1C433ED for ; Wed, 28 Apr 2021 17:52:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3222C610A5 for ; Wed, 28 Apr 2021 17:52:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3222C610A5 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 82A1B6B0036; Wed, 28 Apr 2021 13:52:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7DA106B006E; Wed, 28 Apr 2021 13:52:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 653386B0070; Wed, 28 Apr 2021 13:52:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0182.hostedemail.com [216.40.44.182]) by kanga.kvack.org (Postfix) with ESMTP id 437B46B0036 for ; Wed, 28 Apr 2021 13:52:51 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E2FCD1E1F for ; Wed, 28 Apr 2021 17:52:50 +0000 (UTC) X-FDA: 78082521300.03.EC065EF Received: from mail.skyhub.de (mail.skyhub.de [5.9.137.197]) by imf04.hostedemail.com (Postfix) with ESMTP id B4AA43C4 for ; Wed, 28 Apr 2021 17:52:38 +0000 (UTC) Received: from zn.tnic (p200300ec2f0c1700f2769e812f937597.dip0.t-ipconnect.de [IPv6:2003:ec:2f0c:1700:f276:9e81:2f93:7597]) (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 2BB431EC0242; Wed, 28 Apr 2021 19:52:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1619632359; 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=uNuSRpt+Nn8E1Ie1R3utO4unCBUpAQGFbeK/Hl3lbwA=; b=f66CoGete6UVNGpfGs/Fm2JVHxdlkjHe22uMROZcu8rr0x2vEvFY/1wCWVsC5/+qsG2d20 OC+P3YZqbuPB/ykR8uGOFeBXsaI3T1EXVnAIl5+CHOile1nSbGtjmvFoWjfMD3TKeplfm3 J6jPvXnOnMA5cMOUbp3RdC1kckLGAZU= Date: Wed, 28 Apr 2021 19:52:38 +0200 From: Borislav Petkov To: Yu-cheng Yu 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 22/30] x86/cet/shstk: Add user-mode shadow stack support Message-ID: References: <20210427204315.24153-1-yu-cheng.yu@intel.com> <20210427204315.24153-23-yu-cheng.yu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210427204315.24153-23-yu-cheng.yu@intel.com> X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: B4AA43C4 X-Stat-Signature: ffemihwz4ohdcappo9t3wtht69jsib9b Received-SPF: none (alien8.de>: No applicable sender policy available) receiver=imf04; identity=mailfrom; envelope-from=""; helo=mail.skyhub.de; client-ip=5.9.137.197 X-HE-DKIM-Result: invalid/invalid (public key: DNS error: SERVFAIL) X-HE-Tag: 1619632358-445172 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 Tue, Apr 27, 2021 at 01:43:07PM -0700, Yu-cheng Yu wrote: > @@ -535,6 +536,10 @@ struct thread_struct { > > unsigned int sig_on_uaccess_err:1; > > +#ifdef CONFIG_X86_SHADOW_STACK > + struct cet_status cet; A couple of versions ago I said: " struct shstk_desc shstk; or so" but no movement here. That thing is still called cet_status even though there's nothing status-related with it. So what's up? > +static unsigned long alloc_shstk(unsigned long size) > +{ > + struct mm_struct *mm = current->mm; > + unsigned long addr, populate; > + int flags = MAP_ANONYMOUS | MAP_PRIVATE; The tip-tree preferred ordering of variable declarations at the beginning of a function is reverse fir tree order:: struct long_struct_name *descriptive_name; unsigned long foo, bar; unsigned int tmp; int ret; The above is faster to parse than the reverse ordering:: int ret; unsigned int tmp; unsigned long foo, bar; struct long_struct_name *descriptive_name; And even more so than random ordering:: unsigned long foo, bar; int ret; struct long_struct_name *descriptive_name; unsigned int tmp; Please fix it up everywhere. > + mmap_write_lock(mm); > + addr = do_mmap(NULL, 0, size, PROT_READ, flags, VM_SHADOW_STACK, 0, > + &populate, NULL); > + mmap_write_unlock(mm); > + > + return addr; > +} > + > +int shstk_setup(void) > +{ > + unsigned long addr, size; > + struct cet_status *cet = ¤t->thread.cet; > + > + if (!cpu_feature_enabled(X86_FEATURE_SHSTK)) > + return -EOPNOTSUPP; > + > + size = round_up(min_t(unsigned long long, rlimit(RLIMIT_STACK), SZ_4G), PAGE_SIZE); > + addr = alloc_shstk(size); > + if (IS_ERR_VALUE(addr)) > + return PTR_ERR((void *)addr); > + > + cet->shstk_base = addr; > + cet->shstk_size = size; > + > + start_update_msrs(); > + wrmsrl(MSR_IA32_PL3_SSP, addr + size); > + wrmsrl(MSR_IA32_U_CET, CET_SHSTK_EN); > + end_update_msrs(); <---- newline here. > + return 0; > +} > + > +void shstk_free(struct task_struct *tsk) > +{ > + struct cet_status *cet = &tsk->thread.cet; > + > + if (!cpu_feature_enabled(X86_FEATURE_SHSTK) || > + !cet->shstk_size || > + !cet->shstk_base) > + return; > + > + if (!tsk->mm) > + return; Where are the comments you said you wanna add: https://lkml.kernel.org/r/b05ee7eb-1b5d-f84f-c8f3-bfe9426e8a7d@intel.com ? > + > + while (1) { > + int r; > + > + r = vm_munmap(cet->shstk_base, cet->shstk_size); int r = vm_munmap... > + > + /* > + * vm_munmap() returns -EINTR when mmap_lock is held by > + * something else, and that lock should not be held for a > + * long time. Retry it for the case. > + */ > + if (r == -EINTR) { > + cond_resched(); > + continue; > + } > + break; > + } vm_munmap() can return other negative error values, where are you handling those? > + > + cet->shstk_base = 0; > + cet->shstk_size = 0; > +} > + > +void shstk_disable(void) > +{ > + struct cet_status *cet = ¤t->thread.cet; Same question as before: what guarantees that current doesn't change from under you here? One of the worst thing to do is to ignore review comments. I'd strongly suggest you pay more attention and avoid that in the future. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette