Message-ID: <a1b14f33-5142-8cab-3b5f-4cc79b62091e@intel.com>
Date:   Mon, 6 Dec 2021 13:20:36 -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:     Jarkko Sakkinen <jarkko@kernel.org>,
        Andy Lutomirski <luto@kernel.org>
CC:     <dave.hansen@linux.intel.com>, <tglx@linutronix.de>,
        <bp@alien8.de>, <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>
 <2f6b04dd8949591ee6139072c72eb93da3dd07b0.1638381245.git.reinette.chatre@intel.com>
 <db9b7bc9-fdca-4dd2-2c3f-3b7354c165bb@kernel.org> <YawAWmodeNaUbzV8@iki.fi>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <YawAWmodeNaUbzV8@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?eTM4TG52TTROZ092RkVRUjAyOWlSUTBqVHpBMmpka09zZFRjUUdXVjA2ejNi?=
 =?utf-8?B?L1kyazYvQmdnK0o2RUNCQnluOElyOGNjQU5Lc1MrWDQ1R1lnZ2k2ZzlEMXZG?=
 =?utf-8?B?cTBNU1U3YVgzY3ZQM0hRTFh0UVI1K052QjhCK0FWQUU3ZDJmTENLcUxqM2Np?=
 =?utf-8?B?V2dvYnRoNnVjQ3dqeldONVZMdHVSVCtCQU84UFNsRER6N3FBUVkrbm92c3FX?=
 =?utf-8?B?TjhkSVJlMTh1SGYyK1Rpa0ljODVLSkNtV3BXcmFKZTBMN0oxMHE2c3JneHpB?=
 =?utf-8?B?SmNlZ0FMYjVlTVUraDVqQTUzSlZVY2RoR2haVTJnN3hKQnRLL09QRHNCNUx2?=
 =?utf-8?B?cHFPVDRmb0xOMVBobUR3TnQ0ZUIxS2YyZjFoV0N4emphMXJkSkJXNlRtdVU0?=
 =?utf-8?B?RjBUY1hLdU05WjNFbm1TT2xoWmdOYUd5N0tuaFY1cC9HYTVGRFF0MzQ1OXpX?=
 =?utf-8?B?TVdhZ0x2VFBsdk1uVURFT3ZQeHNqWXpheFd5VXdiaVRzbTJlRDVhTVNFbmQ3?=
 =?utf-8?B?UUdxdDhuR1JFRjYxaFZ3RW1tUldQOHh6c24yemZBVEZLVnkvTS9JRHl4dThv?=
 =?utf-8?B?amZQUW1kYWZ2a1FMTHpjMGNZMUZtZ2wwOEhTbVp6b3dEVStTeDVkZVFZQnBm?=
 =?utf-8?B?a3JWQlBhSnpKWGhpVDVrME1xOUNuM1Blb3ZhQldPeWVTcXB1cWcySTAzRlRK?=
 =?utf-8?B?dk10VktBenUzZHdxZWRmZ3Y5dFZTdTdFNTczNFR3Ui9UbFZyeTlvbjNNck53?=
 =?utf-8?B?eFNSRnRMbUt6bVZjaGVMRGVEU0dRMVg3UjFPTSttM20wL1VxaFhoOTVVeG9v?=
 =?utf-8?B?eEV1NGpERCttNVVZOXpFN1YyMmlGNUVqaUtFYm96TmYvTlltckxLUXZqbFVw?=
 =?utf-8?B?c2xpWVBLNmIvRGlSdEVNU1hLekkycDFVeDNQOXF1MjRFM1c2R1M3Q0Rhc2hk?=
 =?utf-8?B?b0wvalBSRDhBd01wa1RRbnovRTQ5ZVM5RytpYXNpeTBjTUNEbTJFNkRQOWho?=
 =?utf-8?B?S2ZNNllaVzRhYnNhVzJIZXdzYzdJQ25VV3lEWUJiUEZ1MHBGLzRGSStON1pi?=
 =?utf-8?B?L0lWUFZpbGFSSk1Nd1daZHZ4N3FmUXhPQTFoU2RWS1EvNWRqTW04WXNQTDIr?=
 =?utf-8?B?RVBDeHNNbmFWMEtRaUVvZzUxN0Eyd1VXZlcvTDg3RnJCMnpwOXZQZ1dRMytI?=
 =?utf-8?B?aERVRTdQRnJMNis2Q1NnMTBydFhsejdQQWdlYkFiS0N3Q0ptVFR0UDlYaUhX?=
 =?utf-8?B?QzRwWC91VW9aa2ZjRXR2a0o5cGJDT0dEYWJwR2JhY1hxL1VTVlNPRDNGSFg2?=
 =?utf-8?B?cTdmTVRrK3c0bG8yWnFvcDU1dCtiSGZKT1lZcndlNzZnVHh1MzhReXFmWktL?=
 =?utf-8?B?ajUyVmtjOEFCTi9ZVW9lMitsQWo5T2lWYU5WSzlZejJnZjRCeEZ3akdaOVdY?=
 =?utf-8?B?dml2cnhBQjRIcjlxcWZ0aWR5TTRhS2FLV0wvenhUcVNMdHVKTGNSWS9NaEJW?=
 =?utf-8?B?VjIzMWI4VU1PbjQ2TFlIYzNlakIybExUOFk3U3JER3hCNUl0S3lOT1pPNEhs?=
 =?utf-8?B?b1BBdGllUUoyVjljRUsyTzEzZldJbStTT0VBZTVrSVo4WHlvTkRRc2FGVFlr?=
 =?utf-8?B?RUVkRUJBSEJqVjQyTm9UZ3JOQWtBSzFKRkZFZXQzSEptTmJBeWhDVzhFWXRE?=
 =?utf-8?B?VzU0emdtZHJETzFOcm9HUG5xUms0MTVHLzlJM0hRaHNrLzlEeDcrQ21iaHky?=
 =?utf-8?B?aFR6UkJ0bHZCOUFFczFLbXNRRlEwOXZsSXpmNU9jYWxrRDhzYUlPUEZZWWNF?=
 =?utf-8?B?YnZ2U0cwWmZKSkhqWFNwalc2VDNoalVnSUdPNmpvZFE4Ny9zQnViUzZTaGJo?=
 =?utf-8?B?NnJUbnZ3dTRDMzNBMWl3K3hYNjZvZHkxU1hROE9majZYb0tMbWZKU0p6bGp3?=
 =?utf-8?B?clFGdlNISVNJT3V4RldmdURKNEhSYTRXelljZWVCdS9DYTJCM3A2WGt5L2hI?=
 =?utf-8?B?MThya0dWTTZpR3ozQU0rTEFsSFcxTUw0cmtKb0lnQ0J0WWN0VFNCK0hvTzA5?=
 =?utf-8?B?dGdnM0dUVWlPc1pOd1AvSUtzNnQrWGJIWnV5Rm95MUdJK0JSWkdZKzdEOHg5?=
 =?utf-8?B?eU54WWovYWMzZlBUZEdsOTdoMUY3R0hSaEV3c0cyeThXNGVCRWRzNE9FbDNI?=
 =?utf-8?B?aUxLUHN0SnIwQktNOTJYZllTOGxFWXU1SFlhQ1l3aDkxMy9LdUJuL1pzUkN4?=
 =?utf-8?Q?XyxUpfKJ6qdDiz0UeJjZkH4rM4fdiqigWhr1iCtMjE=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: be18b798-7733-47f7-7335-08d9b8fe4165
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:20:41.0556
 (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: bO8t0MMYpsyGpraSk4EA38sGTQi/+BC3ApZiE8SUDFCNGqhTa32mNmO6w7C+tqj4qgbGjqPpH2wagwS+AAGAjllwAny7Ecd0SuaBi2JXCxI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR11MB3796
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 3:57 PM, Jarkko Sakkinen wrote:
> On Fri, Dec 03, 2021 at 11:28:04AM -0800, 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:
>>
>> The PTE: controlled by complex kernel policy
>>
>> The VMA: with your series, this is entirely controlled by userspace.  I
>> think I'm fine with that.
>>
>> vm_max_prot_bits: populated from secinfo at setup time, unless I missed
>> something that changes it later.  Maybe I'm confused or missed something in
>> one of the patches,
>>
>> vm_run_prot_bits: populated from some combination of ioctls.  I'm entirely
>> lost as to what this is for.
>>
>> EPCM bits: controlled by the guest.  basically useless for any host purpose
>> on SGX2 hardware (with or without kernel support -- the enclave can do
>> ENCLU[EMODPE] whether we like it or not, even on old kernels)
>>
>> So I guess I don't understand the purpose of this patch	or of the rules in
>> the later patches, and I feel like this is getting more complicated than
>> makes sense.
>>
>>
>> Could we perhaps make vm_max_prot_bits dynamic or at least controllable in
>> some useful way?  My initial proposal (years ago) was for vm_max_prot_bits
>> to be *separately* configured at initial load time instead of being inferred
>> from secinfo with the intent being that the user untrusted runtime would set
>> it appropriately.  I have no problem with allowing runtime changes as long
>> as the security policy makes sense and it's kept consistent with PTEs.
> 
> This is a valid question. Since EMODPE exists why not just make things for
> EMODPE, and ignore EMODPR altogether?
> 

I believe that we should support the best practice of principle of least 
privilege - once a page no longer needs a particular permission there 
should be a way to remove it (the unneeded permission).

Reinette