Message-ID: <81308f67-a4d1-1774-f58b-223d4e81f8db@intel.com>
Date:   Mon, 6 Dec 2021 13:16:17 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.3.2
Subject: Re: [PATCH 03/25] x86/sgx: Support VMA permissions exceeding enclave
 permissions
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>
CC:     <dave.hansen@linux.intel.com>, <tglx@linutronix.de>,
        <bp@alien8.de>, <luto@kernel.org>, <mingo@redhat.com>,
        <linux-sgx@vger.kernel.org>, <x86@kernel.org>, <seanjc@google.com>,
        <kai.huang@intel.com>, <cathy.zhang@intel.com>,
        <cedric.xing@intel.com>, <haitao.huang@intel.com>,
        <mark.shanahan@intel.com>, <hpa@zytor.com>,
        <linux-kernel@vger.kernel.org>
References: <cover.1638381245.git.reinette.chatre@intel.com>
 <7e622156315c9c22c3ef84a7c0aeb01b5c001ff9.1638381245.git.reinette.chatre@intel.com>
 <Yavq83gZzvkVaDqq@iki.fi> <YavrSFDJBGqe7K46@iki.fi>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <YavrSFDJBGqe7K46@iki.fi>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZTQwdjZVTmQxWVNRN3Jubi9XOGdkbzdiNVdpYnNCK3BVemVTU2lycDNSMy8r?=
 =?utf-8?B?ZzFBM2V3ZFN2dkpiVmZNczI4b21wOVhpQ1VWQmFuYVd4dGVnUWxYbUY4M0kz?=
 =?utf-8?B?Tkw4WFJ4MWRzVXFnVlNPS2xZT0Yycmdqem5WRE1jRVgwSjg3WlFkeldoUWND?=
 =?utf-8?B?ZnZCZFN4c3RIMW5lTlp1Uzc0ZHhVZlNKbjZMR2Jndng2M2dIRDBtaWVYaVJj?=
 =?utf-8?B?eHBVZmtHMERaYlJmanVyVDVlMDUvNTNIbUFWelBRQk1sNlRMQ0F3VzlpN2tC?=
 =?utf-8?B?ZWdWSDZJSVpkVkNGZ1hESFNmb1VLRzAxSEUvbkdjdjlyd0wzdU9oVkV2UXYx?=
 =?utf-8?B?cDJzQU84SjBBUjQ3aUg5MDFyUlRpYktrZWF1U1lYckVxd2hzRGpiczV3Wmhv?=
 =?utf-8?B?YnFZcDlOdGcyWGorZUJnelhLN0ZnVUVneU5uZVlYdWlkblkvbFZ3QUpQSWh2?=
 =?utf-8?B?ZnFvZFBXU1lLVlJ4TjIxYm9tUVJNWDNOcW4vdlNGTHBkaEExZWVSbUFobnRk?=
 =?utf-8?B?cHB4TUZVK0VpOUs1NERhdUZUSmZtMnM2QXV3a3l2d0tzMmVNb0oxVDR2UmZu?=
 =?utf-8?B?NDRjZExHUDBPMUdwMlJtL05sbm9ETXV2Z0NFWDZWVUlKKy95bWRKckpGbzhX?=
 =?utf-8?B?cTd4cVVjb2pOMGVhSkphTy93QUZZUE1wZHNMNkZ6bDhpZFk2UGpETSt5Rys0?=
 =?utf-8?B?eEdEdDl1eERVcldOWlIvbm8zYkZGTlJMTE51NFRwT1JNTVhKWXVxOXhVTURQ?=
 =?utf-8?B?VjN5c2JXanNFa1hwaTUyOUl1Z1lmVVhoMXV1M0pianJhL09FSXRnTzR6M3F3?=
 =?utf-8?B?MHh0cFdpVTVuR3JERHYreTBtdnZHaGZTYXlRUlM3Q29DQ1lmMi9VL2xxYW9y?=
 =?utf-8?B?alRlMHZ3UGtrdkt0UzZTMHhTVjhlVHhnWUI4VGdJSHhDMXpkNmNGQUxUSWI4?=
 =?utf-8?B?TVhFa0NBaDlIZW1IVzFtK2YwN0dZLzI5Zm1aSmxSUFJQWWdJb0FXbFRJbGc1?=
 =?utf-8?B?aHYvdVJPVml5aTloc1pNSWR5cy9KZzRXZmNYbkxWRjQxNXMzRzBhaWRyMXJ4?=
 =?utf-8?B?bmtiNGJ0OFVzcjYxZ0dRazZKQSs4aU9taGd5dlNyb1hkTmVKbnNoQXc3S2xL?=
 =?utf-8?B?cGhPcGpEOVplUHZ5bHUwYW9reWc1c1NlTkdNaUdGY0dTLzE0UTRTaDNoaFkr?=
 =?utf-8?B?dFRrOXRFd1dTZW9VWktja3FxNFVxZEVqb2lrL0NBdmNvMi92U2ljK0JMamdM?=
 =?utf-8?B?cXM0dzhqMkhnN2tieG1VQ2dRT0R2OVZxZHdVbWsyTEVaSFdpUExEUS9FNU9N?=
 =?utf-8?B?SDlRS1QzVGtjRnRScXVFYkc2WFVaSWdyOWkrVDZ2VWFkb0t6RHNvQ0dTakRT?=
 =?utf-8?B?U0FEVmRwN1dBLzk5KzI0VGRMaE5lbDl4bTB3bmZWcUhTUVNVVGdDeDNVSHFL?=
 =?utf-8?B?NUViNG90bzR6Z0ZPUHEwMUEvSnRHMXVQVWRwSi9YanNVaStCdWlsQ3lRV3RF?=
 =?utf-8?B?d1JvbklPVGErZSs4dHRhMlRlbCs3L3ZGVmxOek43VFpnMTExaTB2bTZueVAy?=
 =?utf-8?B?Tmx3emFuQmZ3d0w2TzZlY1JCRFZBN3IrYUJ3UFJaRm9pYTNRWTFzSXdIdlBm?=
 =?utf-8?B?by96TENGTTBIT21wTUdtWXowbURLZHJPNHJnRGdMTEdmNHhvODQ3SklrYW4y?=
 =?utf-8?B?eGE0VnhyUklBbUlUVFFxS1lHZTh2ZnA4OVdoQmVWWnRSMHoxRnk0Rmh6SUsw?=
 =?utf-8?B?QWh4V0N1K0lxNERSOUx2REZwVk1jaThoOG9zYWtXc0RGU2NqU2h3ejFhTm1j?=
 =?utf-8?B?SmNaVVpaQ2RIeFNZM1h3RzRWSkdUK09vdWhqaFR5bklGcHpqcVVlQkswWnJa?=
 =?utf-8?B?QXByRFRIQ3pxYWEyVVV2MTNzaUdHK0FuQnZDclpCcDFFNmJrS0l6djUzVjNl?=
 =?utf-8?B?VTI5ZlMrSXBhM0JSejFIUVFCZ0lpWHFINmZ1NmZFa0w3TVdSYlpKVGdCMnNq?=
 =?utf-8?B?QVQ1VGRsNk4zdzNlc1RDN21BdGVsV3NrbEkwazZTc3l0QVRaZ0Q1bU9oU1A0?=
 =?utf-8?B?RUVCMXlVd3lxWTRGbWpxYXNxbjNRaEFoOC9mYnhPV2phQ0F6ZDhNck5FTmRU?=
 =?utf-8?B?SEs5L1BwT293NGxTNTdXZXZMSHlyOHA5NGpsUlg1aGRNZVVVOHRvMnQrNXE2?=
 =?utf-8?B?ODBCdHVmTnFlbGcyUXBxUzZqcEtSeEZ5QnFwdXFxQk5VY1hGdmhWT0Z5MnRG?=
 =?utf-8?Q?VDj3Epg4+hpGMi2vBYJWLrpzSsuFV6p4QDLi8dJXBA=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6fb720d2-8837-47f2-b134-08d9b8fda6fe
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2021 21:16:21.9015
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 390UZ65mtjCD9xs5QIj9zKWxBDWZjHrhZQLgJZcb6sbnybSEeNHjQ2Q6uon+eYsNxGHQ1T0xtkVUmzOJmpm7QrqU/X846F/DAc1wTceNkjQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5513
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jarkko,

On 12/4/2021 2:27 PM, Jarkko Sakkinen wrote:
> On Sun, Dec 05, 2021 at 12:25:59AM +0200, Jarkko Sakkinen wrote:
>> On Wed, Dec 01, 2021 at 11:23:01AM -0800, Reinette Chatre wrote:
>>> === Summary ===
>>>
>>> An SGX VMA can only be created if its permissions are the same or
>>> weaker than the Enclave Page Cache Map (EPCM) permissions. After VMA
>>> creation this rule continues to be enforced by the page fault handler.
>>>
>>> With SGX2 the EPCM permissions of a page can change after VMA
>>> creation resulting in the VMA exceeding the EPCM permissions and the
>>> page fault handler incorrectly blocking access.
>>>
>>> Enable the VMA's pages to remain accessible while ensuring that
>>> the page table entries are installed to match the EPCM permissions
>>> without exceeding the VMA perms issions.
>>
>> I don't understand what the short summary means in English, and the
>> commit message is way too bloated to make any conclusions. It really
>> needs a rewrite.
>>
>> These were the questions I could not find answer for:
>>
>> 1. Why it would be by any means safe to remove a permission check?

The permission check is redundant for SGX1 and incorrect for SGX2.

In the current SGX1 implementation the permission check in 
sgx_encl_load_page() is redundant because an SGX VMA can only be created 
if its permissions are the same or weaker than the EPCM permissions.

In SGX2 a user is able to change EPCM permissions during runtime (while 
VMA has the memory mapped). A RW VMA may thus originally have mapped an 
enclave page with RW EPCM permissions but since then the enclave page 
may have its permissions changed to read-only. The VMA should still be 
able to read those enclave pages but the check in sgx_encl_load_page() 
will prevent that.

>> 2. Why not re-issuing mmap()'s is unfeasible? I.e. close existing
>>     VMA's and mmap() new ones.

User is not prevented from closing existing VMAs and creating new ones.

> 3. Isn't this an API/ABI break?

Could you please elaborate where you see the API/ABI break? The rule 
that new VMAs cannot exceed EPCM permissions is untouched.

Reinette