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
| ||
|
Date: Thu, 24 Sep 2020 14:11:37 -0500 From: "Haitao Huang" <haitao.huang@...ux.intel.com> To: "Sean Christopherson" <sean.j.christopherson@...el.com>, "Jarkko Sakkinen" <jarkko.sakkinen@...ux.intel.com> Cc: "Andy Lutomirski" <luto@...nel.org>, "X86 ML" <x86@...nel.org>, linux-sgx@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, Linux-MM <linux-mm@...ck.org>, "Andrew Morton" <akpm@...ux-foundation.org>, "Matthew Wilcox" <willy@...radead.org>, "Jethro Beekman" <jethro@...tanix.com>, "Darren Kenny" <darren.kenny@...cle.com>, "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>, asapek@...gle.com, "Borislav Petkov" <bp@...en8.de>, "Xing, Cedric" <cedric.xing@...el.com>, chenalexchen@...gle.com, "Conrad Parker" <conradparker@...gle.com>, cyhanish@...gle.com, "Dave Hansen" <dave.hansen@...el.com>, "Huang, Haitao" <haitao.huang@...el.com>, "Josh Triplett" <josh@...htriplett.org>, "Huang, Kai" <kai.huang@...el.com>, "Svahn, Kai" <kai.svahn@...el.com>, "Keith Moyer" <kmoy@...gle.com>, "Christian Ludloff" <ludloff@...gle.com>, "Neil Horman" <nhorman@...hat.com>, "Nathaniel McCallum" <npmccallum@...hat.com>, "Patrick Uiterwijk" <puiterwijk@...hat.com>, "David Rientjes" <rientjes@...gle.com>, "Thomas Gleixner" <tglx@...utronix.de>, yaozhangx@...gle.com Subject: Re: [PATCH v38 10/24] mm: Add vm_ops->mprotect() On Wed, 23 Sep 2020 08:50:56 -0500, Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com> wrote: > On Tue, Sep 22, 2020 at 09:43:02AM -0700, Sean Christopherson wrote: >> On Tue, Sep 22, 2020 at 08:35:15AM +0300, Jarkko Sakkinen wrote: >> > On Tue, Sep 22, 2020 at 08:30:06AM +0300, Jarkko Sakkinen wrote: >> > > On Mon, Sep 21, 2020 at 02:18:49PM -0700, Sean Christopherson wrote: >> > > > Userspace can add the page without EXEC permissions in the EPCM, >> and thus >> > > > avoid the noexec/VM_MAYEXEC check. The enclave can then do >> EMODPE to gain >> > > > EXEC permissions in the EPMC. Without the ->mprotect() hook, we >> wouldn't >> > > > be able to detect/prevent such shenanigans. >> > > >> > > Right, the VM_MAYEXEC in the code is nested under VM_EXEC check. >> > > >> > > I'm only wondering why not block noexec completely with any >> permissions, >> > > i.e. why not just have unconditional VM_MAYEXEC check? >> > >> > I.e. why not this: >> > >> > static int __sgx_encl_add_page(struct sgx_encl *encl, >> > struct sgx_encl_page *encl_page, >> > struct sgx_epc_page *epc_page, >> > struct sgx_secinfo *secinfo, unsigned long src) >> > { >> > struct sgx_pageinfo pginfo; >> > struct vm_area_struct *vma; >> > struct page *src_page; >> > int ret; >> > >> > vma = find_vma(current->mm, src); >> > if (!vma) >> > return -EFAULT; >> > >> > if (!(vma->vm_flags & VM_MAYEXEC)) >> > return -EACCES; >> > >> > I'm not seeing the reason for "partial support" for noexec partitions. >> > >> > If there is a good reason, fine, let's just then document it. >> >> There are scenarios I can contrive, e.g. loading an enclave from a >> noexec >> filesystem without having to copy the entire enclave to anon memory, or >> loading a data payload from a noexec FS. >> >> They're definitely contrived scenarios, but given that we also want the >> ->mprotect() hook/behavior for potential LSM interaction, supporting >> said >> contrived scenarios costs is "free". > > For me this has caused months of confusion and misunderstanding of this > feature. I only recently realized that "oh, right, we invented this". > > They are contrived scenarios enough that they should be considered when > the workloads hit. > > Either we fully support noexec or not at all. Any "partial" thing is a > two edged sword: it can bring some robustness with the price of > complexity and possible unknown uknown scenarios where they might become > API issue. > > I rather think later on how to extend API in some way to enable such > contrivid scenarios rather than worrying about how this could be abused. > > The whole SGX is complex beast already so lets not add any extra when > there is no a hard requirement to do so. > > I'll categorically deny noexec in the next patch set version. > > /Jarkko There are use cases supported currently in which enclave binary is received via IPC/RPC and held in buffers before EADD. Denying noexec altogether would break those, right?
Powered by blists - more mailing lists