linux-kernel - Re: [PATCH 10/25] x86/sgx: Support enclave page permission changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b271bd29-1e7d-a10c-f71b-533938c77a0c@kernel.org>
Date:   Fri, 3 Dec 2021 16:42:44 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Reinette Chatre <reinette.chatre@...el.com>,
        dave.hansen@...ux.intel.com, jarkko@...nel.org, tglx@...utronix.de,
        bp@...en8.de, mingo@...hat.com, linux-sgx@...r.kernel.org,
        x86@...nel.org
Cc:     seanjc@...gle.com, kai.huang@...el.com, cathy.zhang@...el.com,
        cedric.xing@...el.com, haitao.huang@...el.com,
        mark.shanahan@...el.com, hpa@...or.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/25] x86/sgx: Support enclave page permission changes

On 12/3/21 14:34, Reinette Chatre wrote:
> Hi Andy,
> 
> On 12/3/2021 11:38 AM, Andy Lutomirski wrote:
>> On 12/1/21 11:23, Reinette Chatre wrote:
>>> In the initial (SGX1) version of SGX, pages in an enclave need to be
>>> created with permissions that support all usages of the pages, from the
>>> time the enclave is initialized until it is unloaded. For example,
>>> pages used by a JIT compiler or when code needs to otherwise be
>>> relocated need to always have RWX permissions.
>>>
>>> SGX2 includes two functions that can be used to modify the enclave page
>>> permissions of regular enclave pages within an initialized enclave.
>>> ENCLS[EMODPR] is run from the OS and used to restrict enclave page
>>> permissions while ENCLU[EMODPE] is run from within the enclave to
>>> extend enclave page permissions.
>>>
>>> Enclave page permission changes need to be approached with care and
>>> for this reason this initial support is to allow enclave page
>>> permission changes _only_ if the new permissions are the same or
>>> more restrictive that the permissions originally vetted at the time the
>>> pages were added to the enclave. Support for extending enclave page
>>> permissions beyond what was originally vetted is deferred.
>>>
>>
>> I may well be missing something, but off the top of my head, literally 
>> the only reason that EMODPR needs CPL0 (i.e. ENCLS) is that it 
>> requires a TLB flush IPI to take effect.  (Score one for AMD for being 
>> having superior hardware in this regard.)
> 
> My understanding also is that it is the need for TLB flush that require 
> the privilege but I am trying to get more information here.
> 
>>
>> Given that, I don't see any reason for the EMODPR operation to be 
>> treated as security sensitive -- it just needs to be implemented 
>> correctly.  I don't even see why the host should (or even can!) do any 
>> useful tracking of the EPCM state.
> 
> The OS needs to know the EPCM permissions to be able to install the 
> appropriate PTEs. If the enclave chooses to change the enclave page 
> permissions from within the enclave then user space needs to let the OS 
> know via the SGX_IOC_PAGE_MODP ioctl to ensure that the OS can install 
> correct PTEs in support of the permission change.
> 
> 
>> (But I am confused about one thing: to the extent an enclave actually 
>> needs EMODPR, is there anything in the hardware or anything that the 
>> enclave can do short of actually poking the page from all threads and 
>> confirming that a fault occurs to make sure the OS actually flushed 
>> the TLB?  ISTM a malicious host could attack an enclave by omitting 
>> the TLB flush and then exploiting an enclave but that would have been 
>> mitigated if the flush occurred.)
> 
> When enclave page permissions are restricted it requires the enclave to 
> accept the new permissions from within the enclave by running 
> ENCLU[EACCEPT]. This instruction requires that (it will fail otherwise) 
> the OS completed an ENCLS[ETRACK] on the affected page - essentially 
> ENCLU[EACCEPT] can only succeed if no cached linear-to-physical address 
> mappings are present. The ETRACK flow is elaborate and I attempted to 
> document it in patch 06/25. Essentially, SGX hardware flushes all cached 
> linear-to-physical mappings when an enclave is exited and with ETRACK it 
> can be ensured that all threads that were in an enclave at the time the 
> tracking started (in this case after ENCLS[EMODPR]), have exited.
> 

Does the enclave do something before asking for the ioctl to put the 
page in a state where the tracking is armed?  I read the SDM, but I 
probably read the wrong part of the SDM, and I may have missed this.