Message-ID: <ea7708f6-a4c7-9f00-3c69-39cae0c7ac86@intel.com>
Date:   Fri, 3 Dec 2021 17:14:38 -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 05/25] x86/sgx: Introduce runtime protection bits
Content-Language: en-US
To:     Andy Lutomirski <luto@kernel.org>, <dave.hansen@linux.intel.com>,
        <jarkko@kernel.org>, <tglx@linutronix.de>, <bp@alien8.de>,
        <mingo@redhat.com>, <linux-sgx@vger.kernel.org>, <x86@kernel.org>
CC:     <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>
 <2f6b04dd8949591ee6139072c72eb93da3dd07b0.1638381245.git.reinette.chatre@intel.com>
 <db9b7bc9-fdca-4dd2-2c3f-3b7354c165bb@kernel.org>
 <c0aab85a-f6a1-ed5c-5540-b03778ffe24a@intel.com>
 <4f50b3b3-2cd5-30a4-5715-3350ef2174d9@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <4f50b3b3-2cd5-30a4-5715-3350ef2174d9@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VUt3K3M2djNwaFZ1bU1zS3I5Qno3aWNtSXFJR29ORW9sTkFQSGVhd3pycUpj?=
 =?utf-8?B?ZkN1WW1ld2k0YU56NWdvL21RRGZFZkFjNC9WMXgxbW5WZ3lpYXVyRDVlaGhx?=
 =?utf-8?B?cmpKVlJnQVZzRGJ4YXM4N0Nka1h3aTFzZjlBN1JXNTI4VFY3cUlXdTY0R3hz?=
 =?utf-8?B?RlFVaUJ1WmJqd0pOUTJzS2RvWTVpNlBRdWZ2RVp0eTZ6dUdNLzgzdE1oSGtw?=
 =?utf-8?B?SWNFMU5TRjJsTlkzNnBkb2VzbXVtbHBnQWdva3g1bmUxYVBHa0oyR1AxQXZ3?=
 =?utf-8?B?Y3lZN25BWEhIL2RtWVgrWllHNTVaN3JVZVZuL3B1enl6VEJMZDVxMmFrNng1?=
 =?utf-8?B?UzBoS0VZSW5pMGo1L2FMVytENTYwWFlwdUVxbk8xbWpBWVNzQlg2bEZCcnBR?=
 =?utf-8?B?QXY4NmpQa2d2U3lOTlZDSHVEanR1aTV6a25Ienp3MTJ2MXNpNjFxR3VrQ3Jl?=
 =?utf-8?B?bXJGSUtxMFFEMzVRbjNXR21GaW5IMGN6WVVXOVVnSWd1alE0d0xwaTBlTTVz?=
 =?utf-8?B?QjI5ZEV3cEVPaVVuemthWjJFeFozWGFOdDY2ZDdmOUVJRWUvL1prMXQvaHho?=
 =?utf-8?B?K1Q4NU1pbFdLaEJxUUdxMlhVTHNtd2FmVkhiM0cxQm4wSDdkMDJZVktUM3l0?=
 =?utf-8?B?TVczd1BNbjlCdjR6UmUxQU5mRHY5R3NlUk0xWFpTeVFoNmd5QnpnVkdzMHZn?=
 =?utf-8?B?eExhSS90Z29kblVEcFhYNURQSlZXSFlHL0F1d1hiY291QXJod2g4cDFpQWFX?=
 =?utf-8?B?ZnpBQmNHR05pNFhLUnVtbnBKT0dmMW1zdkMvRXlMRkZiK2hYQmY3Yk4vZ3Vu?=
 =?utf-8?B?MVY4ZEl1MzdVRmxQc3VYeDhIYitCZ2lKaDNwcmc3VVhOeFVEbnRRMGNUa005?=
 =?utf-8?B?UmZmRmw0V1B5TmNZbW5TOFoxSmpRNzhtNTNwUDJKMFVNV3hCb3BTZDZXNzgw?=
 =?utf-8?B?NEphRmx3VTJ0M3JPeWhjL0p0dnB6V3NZRlpDT0RNNW94ZjI1ZzVMVFZDMDFv?=
 =?utf-8?B?MlZCWVcxWFpZQ3V4b1JQRmwxWWdwWmZJd1RoYkY5VmF5YWdqRElRWU5wdW1H?=
 =?utf-8?B?ejlQdlA0UHh5cGEwd281dFVVRUwwUTl3U2d3UHF6aVNDN0EzbytaVjNvQ1Y0?=
 =?utf-8?B?ZkZwVEQ2UGg3OTNVRzNESitWRXd3MjlqZGQzVGFkNGdRNUtBUjZrcTd2T1Vi?=
 =?utf-8?B?eXB3THhHVTU0K0dQbXdHQUNiNEx5bUN3SzJTQk1PVFBUVFgwRXlWUHBEUHpo?=
 =?utf-8?B?RjNBYjI5UFpCRFNrajlnVTBXSGhVV3Y1NGRwOFZBVXBrOTRkcEJBNXYwQXdJ?=
 =?utf-8?B?a0lqTXdmQnFlNE5LU0kvSE5IMXNDdXlCbERmKzlQaVBtMGlCSEdRaVNqRTNQ?=
 =?utf-8?B?dWk2OWlnTjU1SzJ0SElvTkNjYlB3Z2R6VFYyTkdiVlpNRlArTlNSd0FMTVI3?=
 =?utf-8?B?cVlCVVFXbG9TSEpESnFLRDNnTjN5SXliSlhlYkRCRkxlN1l2SU9ENk12cmph?=
 =?utf-8?B?SkkwSEVBSDRCcytIRmw5a1BIdDdpdjQvY2VodFZQZVBiZlRVbUhZRUlpcVpB?=
 =?utf-8?B?dU90WEZPMHgyM2RFWksrMWErTEd1dnY5akNPd1pmK3VWVVlUaUR3VWFGL05t?=
 =?utf-8?B?Z2RnYTJ3QkZmR0MyVjAzSDV4clU4clQzTnRBWDhQc2xSait2V2xCN3QwM2NU?=
 =?utf-8?B?YmplRTk1dFM4N3F3ektCcUNnOTFxQ2FXWWpaQnE1d0dYSmxzQ1N3bXN6VWJ4?=
 =?utf-8?B?LzZ3NmFJRGxUSmdIQUwzYWY3UURka1BnZ3p0V3AvM2lIK3RGYlhhWkR3cmto?=
 =?utf-8?B?S0dFTUs3S2ZvTFRFVko1bm9uVlZaUkpSQURZaERhdXNTbTNmbDFqWmtJcDZ3?=
 =?utf-8?B?THl4SXJ3UnA5Wk9Mc3FhOWREQ05MR2s1cEZ3NU9uL2RzN05mV3hwcTRVdnd3?=
 =?utf-8?B?WkV1Wk5Obm9qUndEZGl5anhBbVFOMTNmMUlNODdONW1GM05VaDVTU1hoWVV5?=
 =?utf-8?B?ZUFER2ZoOExWMjFiUHhOQWk1ZHEwOWlCazFrRzkzbDNteTV6VHpvNVp2YzBx?=
 =?utf-8?B?S0cxeVlycEM1M3lDWTQ1U1EyNGxJRkhBNmNhekw0OXJsS0sxbE1vUkkyY3gr?=
 =?utf-8?B?N01KckdFeEdUaS9aYUNjU1NwbW1UTHcva1RkRUdSa0dZTmNYZmlVTnNUTnlD?=
 =?utf-8?B?emkvY0dGdlVWcEtMK2ZDOExXeHdlVDhxVlBVYmhhc1N3K2o5MHZoZGlRcjh0?=
 =?utf-8?Q?EpavI/nEMl7xgBehv9wpvzDYXPdfVucx8OouMvu3W0=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3930f0bf-3c31-4817-4f95-08d9b6c37413
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Dec 2021 01:14:43.5032
 (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: TTs/4Wvk16wLXPEiTvSaAe78vQTBTLj25BczHu6BI65fJ7h4L20VviksVlH7aqazPGsnv5PYwlpE+icOXv4eB43wn4kfncG/galx0eMbNag=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5241
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.

Could you please elaborate how you envision PTEs should be managed in 
this implementation?

Thank you

Reinette