linux-kernel - Re: [PATCH V5 15/31] x86/sgx: Support restricting of enclave page permissions

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] [day] [month] [year] [list]

Message-ID: <PSAPR04MB4167F203DD60585D538155C0E9419@PSAPR04MB4167.apcprd04.prod.outlook.com>
Date:   Wed, 7 Sep 2022 10:39:28 +0800
From:   zhubojun <bojun.zhu@...look.com>
To:     Reinette Chatre <reinette.chatre@...el.com>
Cc:     bp@...en8.de, cathy.zhang@...el.com, cedric.xing@...el.com,
        dave.hansen@...ux.intel.com, haitao.huang@...el.com, hpa@...or.com,
        jarkko@...nel.org, kai.huang@...el.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-sgx@...r.kernel.org, luto@...nel.org,
        mark.shanahan@...el.com, mingo@...hat.com, seanjc@...gle.com,
        shuah@...nel.org, tglx@...utronix.de, vijay.dhanraj@...el.com,
        x86@...nel.org
Subject: Re: [PATCH V5 15/31] x86/sgx: Support restricting of enclave page
 permissions

Hi, Reinette. Sorry for late reply! Appreciate for you detailed explanation!

> On Sep 2, 2022, at 23:22, Reinette Chatre <reinette.chatre@...el.com> wrote:
> 
> How important is the performance of page permission restriction? How
> about the performance of page type modification?

Enclave applications may need to change its page permissions at runtime.
If they change page permissions frequently, it may introduce substantial
overhead, compared to applications running on native Linux.
(`mprotect()` is more lightweight compared to restricting and extending
page permissions inside enclave) 

But I have not profiled the detailed of the page permission restriction’s
performance. So I’m not sure how much benefit we will gain for performance
if we move the ETRACK flow outside the `for loop`.

> From the hardware perspective, a single ETRACK can be run after
> EMODPR is run on a range of pages.
> 
> Some things to keep in mind when considering making this change:
> 
> Note that the enclave's mutex is obtained and released every time
> an enclave page is modified. This is needed to (a) avoid softlockups
> when modifying a large range of pages and (b) give the reclaimer
> opportunity make space to load pages that will be modified.
> 
> Moving the ETRACK flow out of the for loop implies that the mutex would
> be released between the time the page is modified and ETRACK flow is run
> (with enclave mutex held). It is thus possible for other changes
> to be made or attempted on a page between the time it is modified
> and the ETRACK flow. The possible interactions between different
> page modifications (both initiated from user space and the OS via
> the reclaimer) need to be studied if it is considered to split
> this flow in two parts.
> 
> With the ETRACK flow done while the enclave page being modified is
> loaded there is a guarantee that the SECS page is loaded also. When
> the ETRACK flow is isolated there needs to be changes to ensure
> that the SECS page is loaded.

Thanks for pointing out such case which I have not considered yet!

> It needs to be considered how errors will be communicated to user
> space and how possible inconsistent state could affect user space. In
> support of partial success the ioctl() returns a count indicating
> how many pages were successfully modified. With the configuration
> and ETRACK done per page and their failures handled, the meaning
> of this count is clear. This needs to be considered because it is
> not possible for the kernel to undo an EMODPR. So if all (or some of) the 
> EMODPRs succeed but the final ETRACK fails for some reason then
> the successful EMODPR cannot be undone yet all will be considered
> failed? How should this be reported to user space? Variations may be ok
> since EMODPR can be repeated from what I can tell but I envision scenarios
> where some pages in a range may have their permissions restricted
> (and thus have EPCM.PR set) but which pages have this state is not
> clear to user space. I don't know what would be acceptable here.
> Looking at the EACCEPT flow in the SDM it does not seem as though
> EPCM.PR is one of the EPC page settings that are verified.
> 
> Reinette

I agree with you. It is hard to handle when there the final ETRACK fails
but EMODPR succeeds.

Thanks for showing the case I have not considered but
needs thinking deeply!

BR,
Bojun