Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp3244625pxk; Mon, 28 Sep 2020 12:06:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw3ni1s08D5Xmc+dNHijXVvNnYTMWB6DWi/E/of4YhCVAEdkkg0e/5b6QSyqbOGU+P/3Xfs X-Received: by 2002:a17:906:c447:: with SMTP id ck7mr244659ejb.358.1601319982728; Mon, 28 Sep 2020 12:06:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1601319982; cv=none; d=google.com; s=arc-20160816; b=ZHX/kARxK1urOjTJD6Px5/+IFAz9XCbOjWPu5fpAlKGgFrkBIbYTdmhI5MI5ZhsNRQ ZdxWG4VKo0AeTRh5JpmVI7GI3L6rqv+9UO0LcuXE9+bf4bqmjUfrbLGtHZM0Aq46nJIg nFeXjp5OfIbYgkKGTc78vX+t4jyNq5bnbeIM8YTNjMrd/y6UHJ3TfXG9sv485UlEL9p9 73lKYpAxmUWfz4d6URbvqd2/48hgRF1Wu6IpvB0GaY4QXbZRNOnWc7LXj4uqFH/gBjus F6jBHwuxCzFDOz1h+kwtOugM9fI6hyU8h2W0qn8ms3slmgAk6lrFFlYP8AQ4adG6q3Ms oQ/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:ironport-sdr:ironport-sdr; bh=oAMqPiEkFPU8M9IRgn5PWeFFlH53JK6wc/Tr00yUcO4=; b=ZGvjobZd2krhJcv6eM488dsnO+SSvI+qmhetDAGwfOHE8jT8+Rdt7FxE7WSLr1DxU5 h5S9DST9rLKwXAg2tsM4CwkpFE7yHxPYX+ukYiEqSA+zyiBQ14D034jbtkTR681jfDkr mN2NNdSnt0nAp7S1pB2yGYmlpEiPHxUa7K+gQ+p2ReSQcSy4YXwGuha/DSFuS7q0G/Kp 4lcC/RbynUQpUukAYlGupiQ3Oq9+Cmvj8TwXxVRnaERQd2BlFbXgOVJdZFuHEFRqUut8 PXorp+LqijoKCk5nXmly12ltmNCEkYubt819eKBAX5byT4SD2xOifVn6HgvO0WndU4rU BgGw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t12si1093807eji.422.2020.09.28.12.05.59; Mon, 28 Sep 2020 12:06:22 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726578AbgI1TEc (ORCPT + 99 others); Mon, 28 Sep 2020 15:04:32 -0400 Received: from mga03.intel.com ([134.134.136.65]:27306 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726310AbgI1TEc (ORCPT ); Mon, 28 Sep 2020 15:04:32 -0400 IronPort-SDR: Db4LpqT5s5y8ANUINDsZFMGI8JsRKk2jHRGBHJHMD7qgLQeXcX/LgJ3Kv6VTmwxi8FKAS30X+B wmKEtlMnT87g== X-IronPort-AV: E=McAfee;i="6000,8403,9758"; a="162104913" X-IronPort-AV: E=Sophos;i="5.77,315,1596524400"; d="scan'208";a="162104913" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2020 12:04:27 -0700 IronPort-SDR: XmEAeOL/Im4mDpKC+6Gmdg8kzbb4nmZMIW/ZRA58CEKjr+/4kaCzRdQbwBHUEtZbZVXRnSYZiQ S9D+NDksRl4Q== X-IronPort-AV: E=Sophos;i="5.77,315,1596524400"; d="scan'208";a="338299546" Received: from yyu32-mobl1.amr.corp.intel.com (HELO [10.212.43.157]) ([10.212.43.157]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2020 12:04:25 -0700 Subject: Re: [PATCH v13 8/8] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch Tracking for vsyscall emulation To: Andy Lutomirski Cc: X86 ML , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , LKML , "open list:DOCUMENTATION" , Linux-MM , linux-arch , Linux API , Arnd Bergmann , Balbir Singh , Borislav Petkov , 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 References: <99B32E59-CFF2-4756-89BD-AEA0021F355F@amacapital.net> From: "Yu, Yu-cheng" Message-ID: <054bd574-1566-2be4-b542-884500b7319d@intel.com> Date: Mon, 28 Sep 2020 12:04:24 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/28/2020 10:37 AM, Andy Lutomirski wrote: > On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu wrote: >> >> On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote: >>>> On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng wrote: >> + >> + cet = get_xsave_addr(&fpu->state.xsave, XFEATURE_CET_USER); >> + if (!cet) { >> + /* >> + * This is an unlikely case where the task is >> + * CET-enabled, but CET xstate is in INIT. >> + */ >> + WARN_ONCE(1, "CET is enabled, but no xstates"); > > "unlikely" doesn't really cover this. > >> + fpregs_unlock(); >> + goto sigsegv; >> + } >> + >> + if (cet->user_ssp && ((cet->user_ssp + 8) < TASK_SIZE_MAX)) >> + cet->user_ssp += 8; > > This looks buggy. The condition should be "if SHSTK is on, then add 8 > to user_ssp". If the result is noncanonical, then some appropriate > exception should be generated, probably by the FPU restore code -- see > below. You should be checking the SHSTK_EN bit, not SSP. The code now checks if shadow stack is on (yes, it should check SHSTK_EN bit, I will fix it.), then adds 8 to user_ssp. If the result is canonical, then it sets the corresponding xstate. If the resulting address is not canonical, the kernel does not know what the address should be either. I think the best action to take is doing nothing about the shadow stack pointer, and let the application return and get a control protection fault. The application should have not got into such situation in the first place; if it does, it should fault. > > Also, can you point me to where any of these canonicality rules are > documented in the SDM? I looked and I can't find them. The SDM is not very explicit. It should have been. > > This reminds me: this code in extable.c needs to change. > > __visible bool ex_handler_fprestore(const struct exception_table_entry *fixup, > struct pt_regs *regs, int trapnr, > unsigned long error_code, > unsigned long fault_addr) > { > regs->ip = ex_fixup_addr(fixup); > > WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing > FPU registers.", > (void *)instruction_pointer(regs)); > > __copy_kernel_to_fpregs(&init_fpstate, -1); > > Now that we have supervisor states like CET, this is buggy. This > should do something intelligent like initializing all the *user* state > and trying again. If that succeeds, a signal should be sent rather > than just corrupting the task. And if it fails, then perhaps some > actual intelligence is needed. We certainly should not just disable > CET because something is wrong with the CET MSRs. > Yes, but it needs more thought. Maybe a separate patch and more discussion? Yu-cheng