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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham 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 11C7BC433E6 for ; Thu, 18 Mar 2021 19:06:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8858964F1F for ; Thu, 18 Mar 2021 19:06:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8858964F1F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CA1076B0070; Thu, 18 Mar 2021 15:06:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C2A6C6B0071; Thu, 18 Mar 2021 15:06:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7C886B0072; Thu, 18 Mar 2021 15:06:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0106.hostedemail.com [216.40.44.106]) by kanga.kvack.org (Postfix) with ESMTP id 84DEF6B0070 for ; Thu, 18 Mar 2021 15:06:09 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 4128C62FF for ; Thu, 18 Mar 2021 19:06:09 +0000 (UTC) X-FDA: 77933925258.07.6749923 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by imf16.hostedemail.com (Postfix) with ESMTP id 226948019171 for ; Thu, 18 Mar 2021 19:06:02 +0000 (UTC) IronPort-SDR: 44hIyozMWB13e16iW+84TwdrHtNBQm6ijEW9wpvJ1u6fm8M4l/w2QGMzjbADMGwdOcoGWWdNHf pqYSM6IJBCyw== X-IronPort-AV: E=McAfee;i="6000,8403,9927"; a="189845627" X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="189845627" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2021 12:06:00 -0700 IronPort-SDR: hqYeyuc91+tiTRuC7ikztwLhEgKS2tALLRwJSbZ64YO5K2tylKZBX7tsRUFeWimmlOPHqcL6yO P5Jcq/OKHeVg== X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="389358155" Received: from yyu32-mobl1.amr.corp.intel.com (HELO [10.209.36.121]) ([10.209.36.121]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2021 12:05:58 -0700 Subject: Re: [PATCH v23 22/28] x86/cet/shstk: User-mode shadow stack support To: Borislav Petkov 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 References: <20210316151054.5405-1-yu-cheng.yu@intel.com> <20210316151054.5405-23-yu-cheng.yu@intel.com> <20210318123215.GE19570@zn.tnic> From: "Yu, Yu-cheng" Message-ID: Date: Thu, 18 Mar 2021 12:05:58 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210318123215.GE19570@zn.tnic> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Stat-Signature: iy5n9mjog3ghfdeik5u81q9wudb6rw13 X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 226948019171 Received-SPF: none (intel.com>: No applicable sender policy available) receiver=imf16; identity=mailfrom; envelope-from=""; helo=mga09.intel.com; client-ip=134.134.136.24 X-HE-DKIM-Result: none/none X-HE-Tag: 1616094362-205773 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 3/18/2021 5:32 AM, Borislav Petkov wrote: >> Subject: Re: [PATCH v23 22/28] x86/cet/shstk: User-mode shadow stack support > ^ > Add > > On Tue, Mar 16, 2021 at 08:10:48AM -0700, Yu-cheng Yu wrote: >> Introduce basic shadow stack enabling/disabling/allocation routines. >> A task's shadow stack is allocated from memory with VM_SHSTK flag and has >> a fixed size of min(RLIMIT_STACK, 4GB). >> >> Signed-off-by: Yu-cheng Yu >> Reviewed-by: Kees Cook >> --- >> arch/x86/include/asm/cet.h | 28 ++++++ >> arch/x86/include/asm/processor.h | 5 ++ >> arch/x86/kernel/Makefile | 2 + >> arch/x86/kernel/cet.c | 147 +++++++++++++++++++++++++++++++ [...] >> +void cet_free_shstk(struct task_struct *tsk) >> +{ >> + struct cet_status *cet = &tsk->thread.cet; >> + >> + if (!static_cpu_has(X86_FEATURE_SHSTK) || > > cpu_feature_enabled and as above. > >> + !cet->shstk_size || !cet->shstk_base) >> + return; >> + >> + if (!tsk->mm || tsk->mm != current->mm) >> + return; > > You're operating on current here merrily but what's protecting all those > paths operating on current from getting current changed underneath them > due to scheduling? IOW, is preemption safely disabled in all those > paths ending up here? Good thought. Indeed, this looks like scheduling would bring some trouble. However, when this instance is running, the current task must be current, context switch or not. The purpose of this check is described below. When fork() fails, it calls exit_thread(), then cet_free_shstk(). Normally the child tsk->mm != current->mm (parent). There is no need to free shadow stack. For CLONE_VM, however, the kernel has already allocated a shadow stack for the child and needs to free it because fork() failed. Maybe I would add comments here. > >> + >> + while (1) { > > Uuh, an endless loop. What guarantees we'll exit it relatively timely... > >> + int r; >> + >> + r = vm_munmap(cet->shstk_base, cet->shstk_size); >> + >> + /* >> + * Retry if mmap_lock is not available. >> + */ >> + if (r == -EINTR) { >> + cond_resched(); > > ... that thing? If vm_munmap() returns -EINTR, mmap_lock is held by something else. That lock should not be held forever. For other types of error, the loop stops. > >> + continue; >> + } >> + >> + WARN_ON_ONCE(r); >> + break; >> + } >> + >> + cet->shstk_base = 0; >> + cet->shstk_size = 0; >> +} >> -- >> 2.21.0 >> >