[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67993dd0-827e-5f08-d853-4fb0273adb5f@kernel.org>
Date: Sat, 4 Dec 2021 09:56:39 -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 05/25] x86/sgx: Introduce runtime protection bits
On 12/3/21 17:14, Reinette Chatre wrote:
> Hi Andy,
>
> On 12/3/2021 4:38 PM, Andy Lutomirski wrote:
>> On 12/3/21 14:12, Reinette Chatre wrote:
>>> Hi Andy,
>>>
>>> On 12/3/2021 11:28 AM, Andy Lutomirski wrote:
>>>> On 12/1/21 11:23, Reinette Chatre wrote:
>>>>> Enclave creators declare their paging permission intent at the time
>>>>> the pages are added to the enclave. These paging permissions are
>>>>> vetted when pages are added to the enclave and stashed off
>>>>> (in sgx_encl_page->vm_max_prot_bits) for later comparison with
>>>>> enclave PTEs.
>>>>>
>>>>
>>>> I'm a bit confused here. ENCLU[EMODPE] allows the enclave to change
>>>> the EPCM permission bits however it likes with no oversight from the
>>>> kernel. So we end up with a whole bunch of permission masks:
>>>
>>> Before jumping to the permission masks I would like to step back and
>>> just confirm the context. We need to consider the following three
>>> permissions:
>>>
>>> EPCM permissions: the enclave page permissions maintained in the SGX
>>> hardware. The OS is constrained here in that it cannot query the
>>> current EPCM permissions. Even so, the OS needs to ensure PTEs are
>>> installed appropriately (we do not want a RW PTE for a read-only
>>> enclave page)
>>
>> Why not? What's wrong with an RW PTE for a read-only enclave page?
>>
>> If you convince me that this is actually important, then I'll read all
>> the stuff below.
>
> Perhaps it is my misunderstanding/misinterpretation of the current
> implementation? From what I understand the current requirement, as
> enforced in the current mmap(), mprotect() as well as fault() hooks, is
> that mappings are required to have identical or weaker permission than
> the enclave permission.
The current implementation does require that, but for a perhaps
counterintuitive reason. If a SELinux-restricted (or similarly
restricted) process that is *not* permitted to do JIT-like things loads
an enclave, it's entirely okay for it to initialize RW enclave pages
however it likes and it's entirely okay for it to initialize RX (or XO
if that ever becomes a thing) enclave pages from appropriately files on
disk. But it's not okay for it to create RWX enclave pages or to
initialize RX enclave pages from untrusted application memory. [0]
So we have a half-baked implementation right now: the permission to
execute a page is decided based on secinfo (max permissions) when the
enclave is set up, and it's enforced at the PTE level. The PTE
enforcement is because, on SGX2 hardware, the enclave can do EMODPE and
bypass any supposed restrictions in the EPCM.
The only coupling between EPCM and PTE here is that the max_perm is
initialized together with EPCM, but it didn't have to be that way.
An SGX2 implementation needs to be more fully baked, because in a
dynamic environment enclaves need to be able to use EMODPE and actually
end up with permissions that exceed the initial secinfo permissions. So
it needs to be possible to make a page that starts out R (or RW or
whatever) but nonetheless has max_perm=RWX so that the enclave can use a
combination of EMODPE and (ioctl-based) EMODPR to do JIT. So I think
you should make it possible to set up pages like this, but I see no
reason to couple the PTE and the EPCM permissions.
>
> Could you please elaborate how you envision PTEs should be managed in
> this implementation?
As above: PTE permissions may not exceed max_perm, and EPCM is entirely
separate except to the extent needed for ABI compatibility with SGX1
runtimes.
[0] I'm not sure anyone actually has a system set up like this or that
the necessary LSM support is in the kernel. But it's supposed to be
possible without changing the ABI.
Powered by blists - more mailing lists