From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38A2DC56202 for ; Thu, 12 Nov 2020 22:41:18 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A398822227 for ; Thu, 12 Nov 2020 22:41:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="teH4Qyhw" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A398822227 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D76226B005C; Thu, 12 Nov 2020 17:41:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D24056B005D; Thu, 12 Nov 2020 17:41:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD9516B0068; Thu, 12 Nov 2020 17:41:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0231.hostedemail.com [216.40.44.231]) by kanga.kvack.org (Postfix) with ESMTP id 6D10A6B005D for ; Thu, 12 Nov 2020 17:41:16 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 1F3938249980 for ; Thu, 12 Nov 2020 22:41:16 +0000 (UTC) X-FDA: 77477238552.15.night89_1b0d6f12730a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin15.hostedemail.com (Postfix) with ESMTP id F3BB41814B0C7 for ; Thu, 12 Nov 2020 22:41:15 +0000 (UTC) X-HE-Tag: night89_1b0d6f12730a X-Filterd-Recvd-Size: 7510 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf43.hostedemail.com (Postfix) with ESMTP for ; Thu, 12 Nov 2020 22:41:15 +0000 (UTC) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 01BC022250 for ; Thu, 12 Nov 2020 22:41:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1605220874; bh=TIUppCALNwaHlE5rK5c0ydSZxnQCuBpff8wpCFgyPXE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=teH4Qyhw8xsxYmB8MLkP7Of2tY8v+nEIlr8OA0TEL7K2Ue1KY0Z3iTsgrEQY7gA32 h5CTD3PWMJKbTZKMqLiKbfA6b3kT0XE4NVZbPwYBIlxFUdZcAB78PsmaOuZ09rERXX 1YBE1h6O51V7CFjQsaFlVG1nUc8HRuxI+HRSGIes= Received: by mail-wm1-f43.google.com with SMTP id 10so6585619wml.2 for ; Thu, 12 Nov 2020 14:41:13 -0800 (PST) X-Gm-Message-State: AOAM530O21a828edUR15p5OQiX6RWNmf4tMolDNSjCc10D0VXR/kAabI Mo/qvrgFfTnULh6ToTTO3zRr6cWkenGhlrP3V8ri7w== X-Google-Smtp-Source: ABdhPJwjDJ5n6cZYqwYLuSjOEw1F3mMY5MAOPzOD3AxIPaeWfMnaswzLgqo3yGilV0Z1PSDHO7VmDptEJa3o6gGkW78= X-Received: by 2002:a05:600c:22d3:: with SMTP id 19mr44938wmg.21.1605220872356; Thu, 12 Nov 2020 14:41:12 -0800 (PST) MIME-Version: 1.0 References: <20201104145430.300542-1-jarkko.sakkinen@linux.intel.com> <20201104145430.300542-11-jarkko.sakkinen@linux.intel.com> <20201106174359.GA24109@wind.enjellic.com> <20201107150930.GA29530@wind.enjellic.com> <20201112205819.GA9172@wind.enjellic.com> <5c22300c-0956-48ed-578d-00cf62cb5c09@intel.com> In-Reply-To: <5c22300c-0956-48ed-578d-00cf62cb5c09@intel.com> From: Andy Lutomirski Date: Thu, 12 Nov 2020 14:41:00 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v40 10/24] mm: Add 'mprotect' hook to struct vm_operations_struct To: Dave Hansen Cc: "Dr. Greg" , Jarkko Sakkinen , X86 ML , linux-sgx@vger.kernel.org, LKML , Sean Christopherson , Linux-MM , Andrew Morton , Matthew Wilcox , Jethro Beekman , Darren Kenny , Andy Shevchenko , asapek@google.com, Borislav Petkov , "Xing, Cedric" , chenalexchen@google.com, Conrad Parker , cyhanish@google.com, "Huang, Haitao" , "Huang, Kai" , "Svahn, Kai" , Keith Moyer , Christian Ludloff , Andrew Lutomirski , Neil Horman , Nathaniel McCallum , Patrick Uiterwijk , David Rientjes , Thomas Gleixner , yaozhangx@google.com, Mikko Ylinen Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Nov 12, 2020 at 1:31 PM Dave Hansen wrote: > > On 11/12/20 12:58 PM, Dr. Greg wrote: > > @@ -270,11 +270,10 @@ static int sgx_vma_mprotect(struct vm_area_struct *vma, > > struct vm_area_struct **pprev, unsigned long start, > > unsigned long end, unsigned long newflags) > > { > > - int ret; > > + struct sgx_encl *encl = vma->vm_private_data; > > > > - ret = sgx_encl_may_map(vma->vm_private_data, start, end, newflags); > > - if (ret) > > - return ret; > > + if ( test_bit(SGX_ENCL_INITIALIZED, &encl->flags) ) > > + return -EACCES; > > > > return mprotect_fixup(vma, pprev, start, end, newflags); > > } > > This rules out mprotect() on running enclaves. Does that break any > expectations from enclave authors, or take away capabilities that folks > need? It certainly prevents any scheme in which an enclave coordinates with the outside world to do W-and-then-X JIT inside. I'm also not convinced it has any real effect at all unless there's some magic I missed to prevent someone from using mmap(2) to effectively change permissions. Everyone, IMO this SGX1 - vs - SGX2 - vs - EDMM discussion is entirely missing the point and is a waste of everyone's time. Let's pretend we're building a system that has nothing to do with SGX and requires no special hardware support at all. It works like this: A user program opens /dev/xyz and gets back an fd that represents 16 MB of memory. The user program copies some data from disk (or network or whatever) into fd (using write(2) or ioctl(2) or mmap(2) and memcpy) and then mmaps some of the fd as R and some as RW and some as RX, and then the user program jumps into the RX mapping. If we replace /dev/xyz with /dev/zero, then this simply does not work under a reasonably strict W^X policy -- a lot of people think it's quite reasonable for an OS to prevent a user program from obtaining an X mapping containing anything other than a mapping from a file on disk. To solve this, we can do one of at least three things: a) You can't use /dev/xyz unless you have permission to create WX memory or to at least create W memory and then change it to X. b) You can do whatever you want with /dev/xyz, and LSM policies are blatantly violated as a result. c) The /dev/xyz API is clever and tracks, page-by-page, whether the user intends to ever write and/or execute that page, and behaves accordingly. This patchset takes the approach (c). The actual clever policy isn't here yet, and we don't really know whether it will ever appear, but the API is set up to enable such a policy to be written. This appears to be a win for everyone, since the code is pretty clean and the API is straightforward. Now, back to SGX. There are only two things that are remotely SGX-specific here. First, SGX requires this unusual memory model in which there is an executable mapping of (part of) a device node. [0] Second, early SGX hardware had this oddity that the kernel could set a per-backing-page (as opposed to per-PTE) bit to permanently disable X on a given /dev/sgx page. Building a security model around that would have been a hack, and it DOES NOT WORK on new hardware. So can we please stop discussing it? None of the actual interesting parts of this have much to do with SGX per se and have nothing whatsoever to do with EDMM or any other Intel buzzword. Heck, if anyone actually cared to do so, something with essentially the same semantics could probably be built using SEV hardware instead of SGX, and it would have exactly the same issue if we wanted it to work for tasks that didn't have access to /dev/kvm. [0] SGX doesn't *really* require this. We could set things up so that you do mmap(..., MAP_ANONYMOUS, fd, ...) and then somehow introduce that mapping to SGX. I think the result would be too disgusting to seriously consider.