Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 8 Aug 2019 08:40:00 -0700
From:   Sean Christopherson <sean.j.christopherson@intel.com>
To:     Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc:     "Ayoun, Serge" <serge.ayoun@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "nhorman@redhat.com" <nhorman@redhat.com>,
        "npmccallum@redhat.com" <npmccallum@redhat.com>,
        "Katz-zamir, Shay" <shay.katz-zamir@intel.com>,
        "Huang, Haitao" <haitao.huang@intel.com>,
        "andriy.shevchenko@linux.intel.com" 
        <andriy.shevchenko@linux.intel.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "Svahn, Kai" <kai.svahn@intel.com>, "bp@alien8.de" <bp@alien8.de>,
        "josh@joshtriplett.org" <josh@joshtriplett.org>,
        "luto@kernel.org" <luto@kernel.org>,
        "Huang, Kai" <kai.huang@intel.com>,
        "rientjes@google.com" <rientjes@google.com>,
        "Xing, Cedric" <cedric.xing@intel.com>
Subject: Re: [PATCH v21 16/28] x86/sgx: Add the Linux SGX Enclave Driver
Message-ID: <20190808154000.GB23156@linux.intel.com>
References: <20190713170804.2340-1-jarkko.sakkinen@linux.intel.com>
 <20190713170804.2340-17-jarkko.sakkinen@linux.intel.com>
 <88B7642769729B409B4A93D7C5E0C5E7C65ABB8D@hasmsx108.ger.corp.intel.com>
 <20190807151534.kxsletvhbn3lno6w@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190807151534.kxsletvhbn3lno6w@linux.intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Aug 07, 2019 at 06:15:34PM +0300, Jarkko Sakkinen wrote:
> On Mon, Jul 29, 2019 at 11:17:57AM +0000, Ayoun, Serge wrote:
> > > +	/* TCS pages need to be RW in the PTEs, but can be 0 in the EPCM. */
> > > +	if ((secinfo.flags & SGX_SECINFO_PAGE_TYPE_MASK) ==
> > > SGX_SECINFO_TCS)
> > > +		prot |= PROT_READ | PROT_WRITE;
> > 
> > For TCS pages you add both RD and WR maximum protection bits.
> > For the enclave to be able to run, user mode will have to change the
> > "vma->vm_flags" from PROT_NONE to PROT_READ | PROT_WRITE (otherwise
> > eenter fails).  This is exactly what your selftest  does.
> 
> Recap where the TCS requirements came from? Why does it need
> RW in PTEs and can be 0 in the EPCM? The comment should explain
> it rather leave it as a claim IMHO.

Hardware ignores SECINFO.FLAGS.{R,W,X} coming from userspace and instead
forces RWX=0.  It does this to prevent software from directly accessing
the TCS.  But hardware still accesses the TCS through a virtual address,
e.g. to allow software to zap the page for reclaim, which means hardware
generates reads and writes to the TCS, i.e. the PTEs need RW permissions.

So, for the EADD ioctl(), it's not unreasonable for userspace to provide
SECINFO.FLAGS.{R,W,X} = 0 for the TCS to match what will actually get
jammed into the EPCM.  Allowing userspace to specify RWX=0 means the
kernel needs to manually add PROT_READ and PROT_WRITE to the allowed prot
bits so that mmap()/mprotect() work as expected.

From the SDM:


(* For TCS pages, force EPCM.rwx bits to 0 and no debug access *)
IF (SCRATCH_SECINFO.FLAGS.PT = PT_TCS)
    THEN
        SCRATCH_SECINFO.FLAGS.R <= 0;
        SCRATCH_SECINFO.FLAGS.W <= 0;
        SCRATCH_SECINFO.FLAGS.X <= 0;
        (DS:RCX).FLAGS.DBGOPTIN <= 0; // force TCS.FLAGS.DBGOPTIN off
        DS:RCX.CSSA <= 0;
        DS:RCX.AEP <= 0;
        DS:RCX.STATE <= 0;
        FI;