Message-ID: <f6a55943-13ef-41ef-609a-6406cffef513@intel.com>
Date:   Mon, 13 Dec 2021 14:10:17 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.4.0
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>
 <a1b14f33-5142-8cab-3b5f-4cc79b62091e@intel.com>
 <a24bc46e4ba8a69938a7f73012019ce0f61005c2.camel@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <a24bc46e4ba8a69938a7f73012019ce0f61005c2.camel@kernel.org>
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?T1pvRVZkMUV0a0VoT2QxYmE5RWQzbEFhbW9VNWp6UHFObTBWdUNPMjlpemN3?=
 =?utf-8?B?dkhuald4ZFczZHMxemRWNEpXcFY2a2ZXeVB3bGg1SVl0WXJPNUNsNFU1QVpR?=
 =?utf-8?B?MDJGbWZzR0tFSmJERUNJZ1VMc0UvVXoxMTRra3pnekFwMmo3ZU5CeXFiZXpZ?=
 =?utf-8?B?Vm1Wc3RBZ1lNL1hVVDJyNlVBeldxNWZjbkczSU5rTzkwL09NNEI3dDNEcjVF?=
 =?utf-8?B?d1p3ZFNhTjNQdzcyWmVwaFpxOXFiOHJHZ05paHVFb1BSRWRBQlVoNHZLczBF?=
 =?utf-8?B?UTljdVprNUZGUGZSRGRyb29iN1IwUlN5azJWNmcrZm9CemZwZTRGNU50MWtN?=
 =?utf-8?B?MUl0eGZhL2NYVnRtWlMwREpWOEVSWjI5eXFxTE5OT0NhS25uNjhnN0wvMmZC?=
 =?utf-8?B?VEEwaVVHU2cxaGxmTnJSTU9yOXhQWDh0S0pra2trT09rejhVZi9pS3VMUHh6?=
 =?utf-8?B?VkEyYVh5U3lzRkd4bzZlTHVRU05uNVQzQnJYMjlRVjRWcnhNL09JMHF0bjJI?=
 =?utf-8?B?WUlveGxVQTJUMkpLZStLck9RSDBqendVZ213TytBTVVqMUNta1BEbkllREZ4?=
 =?utf-8?B?SU5wcHZXRHVOVEowSS9BZHJjSlFqRTVpaCtuOGwvaFBaWDZ2M2VJZDBWY1pX?=
 =?utf-8?B?M0diZVR2S3AyckgzbDhGUmVkUUx1TXliSmhrejgxVVZyby9yVGQ5NDFQTFE0?=
 =?utf-8?B?Lzh2NWNzM0kxOHllbldDWnlUNXVuNUUybC9FWEQzYXdUV2l4V29EOXVQY0V4?=
 =?utf-8?B?bnhFOXB5MXNmWTNLRkhqeGIrRUVDQTdUUmVSZy91MjZFZVVyNGQyQlo1YzlF?=
 =?utf-8?B?ak1FMlNILzhEVE1kVHRyN3M3VGcvYm1kbGdSYXNPQW1pKzc5aXFNczNnS3pB?=
 =?utf-8?B?QUdGOWpnSFUwMndJcUcrUmJoY3doQzFRODdmSHNpYURjclVicVRrYTkzTm9L?=
 =?utf-8?B?amc3R3d0L1lmZERoVFZrci9xMHVIdDNtM3pUaW5CNGNOUyt0L1Y4Q1BSaUk4?=
 =?utf-8?B?UzVUazF0V1J1ZHA1OVpvL3ZsRFJHMXJmMXBFalVxcjFHWk1SZkpxMjRESXR1?=
 =?utf-8?B?TjdqMVZHY2UzdjZMWVN1SmFZN2hRdisvZzRtaDIxTkxsb3pud3ZuNldZbCtl?=
 =?utf-8?B?Y0U3b1U1YkpzZThTWHBsVTZqcXRJblZ6bFNjK21tRXZIdXduZHVKdmhPeEx3?=
 =?utf-8?B?M3FNRnhVZmNSM29pczZNSjM2NkxQckRpUVozOEtDd05MTjRYWUdQdXV3RG85?=
 =?utf-8?B?MXJVSWF6TU83Y1hKQnVjWTdIU21LK0FucGNyZjg4TVErNmwzRE1JRnRoL3lX?=
 =?utf-8?B?R2Jsc1lndjZUTmJvSGtnV25OcTVGbUswbW1qRkI3SGJVUjdDeXdJTVJUS2I2?=
 =?utf-8?B?dGY3aXJKUHQ1WDJxT0l3SW51eU9xNUFKbFBiaTRza3BBbit0blFQSE56U2E0?=
 =?utf-8?B?eXZONHFWVnlTTlc3YU1mNmErTk9Kb3BuT1lacEV5T0FaM2lIUE1mWXkwOGZl?=
 =?utf-8?B?dVRTSXZiQ2pFZXNvVVdPc1FTeE9wM05TeGlWcGVueUdqUUdFWStYTEZCcW82?=
 =?utf-8?B?bU9sbWJ5b2xXOUpqK1Azbk55ck5BS1BHcVA5Qlc2UG5Tcjltbkgzc09SREdE?=
 =?utf-8?B?b1B5bmFsSlBiZHRyaUcrejFnUVZERnRQUDIySDUveHlEcnc5R2VmZzBhWHhr?=
 =?utf-8?B?dlk0QWFHVVpxTWNJMDdZeDc3NUZtNHR6WmhGUXRIT3hrMXJOa1ZkYzl4LzhL?=
 =?utf-8?B?b0taZ0pOZnR2Tkc5TnJ3Q0YyY0pFRlF0bFd1V29HTGNpeHU3SUVVVTYrODlK?=
 =?utf-8?B?WXIxRlRxc28wUEJiRmFjSG00bjNaSDVVY2RvSEh1ZzhsZmNkenRxRG5MMGts?=
 =?utf-8?B?ZlFsSkVFZDVqVUppOFd1UEx5SWM1Z3pyRzJMK0F4ZDg0UjluMU5BM25Tcjd3?=
 =?utf-8?B?QnYvdnlwNU5UajVVdlNrMjl1c0ZVZFk5VDJCc3NFeG9xQ1dNMSt3dXI5R0NT?=
 =?utf-8?B?b1htQ1RGK3YraDhBakZ0V3o4dFJUcXlyaEQ0OVZOZCs3Yzlic0xGbjlIV1FL?=
 =?utf-8?B?cEdlVXFEcTRFcGVLMWVDRHhHWXZpQU04QWFaS2ovMU1rRUJLUTFyTHYzVjMz?=
 =?utf-8?B?MFZiZmJJVWNmeDF0cFBLVGEwL09MbUVSY2d6T25zejgxUml1VlpldmN4YytO?=
 =?utf-8?B?TkVGQ29VUDA3Zy9pVWpvdzg0SjIxZ29WaElHazk3Q1pBUUUzVXVObm9pcnF1?=
 =?utf-8?Q?5G1cV2XxAzcLOpr4KveGr9s/KC7hgoW3V6DS8/NU/k=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: eda6098d-17e0-41a6-a8dd-08d9be855b36
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2021 22:10:22.2610
 (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: VkLfUtkgk7YJVr7CpCsO7k7Wx3QYPfuJeWByYCknNSCGHaltukmHxvRIADAXovCOsTSCU52g3JhYdj1sO9N5mrzzxeqGSki45XTORfG2oXw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2737
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/10/2021 11:42 PM, Jarkko Sakkinen wrote:
> On Mon, 2021-12-06 at 13:20 -0800, Reinette Chatre wrote:
>>> 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).
> 
> What if EMODPR was not used at all, since EMODPE is there anyway?

EMODPR and EMODPE are not equivalent.

EMODPE can only be used to "extend"/relax permissions while EMODPR can 
only be used to restrict permissions.

Notice in the EMODPE instruction reference of the SDM:

(* Update EPCM permissions *)
EPCM(DS:RCX).R := EPCM(DS:RCX).R | SCRATCH_SECINFO.FLAGS.R;
EPCM(DS:RCX).W := EPCM(DS:RCX).W | SCRATCH_SECINFO.FLAGS.W;
EPCM(DS:RCX).X := EPCM(DS:RCX).X | SCRATCH_SECINFO.FLAGS.X;

So, when using EMODPE it is only possible to add permissions, not remove 
permissions.

If a user wants to remove permissions from an EPCM page it is only 
possible when using EMODPR. Notice in its instruction reference found in 
the SDM how it in turn can only be used to restrict permissions:

(* Update EPCM permissions *)
EPCM(DS:RCX).R := EPCM(DS:RCX).R & SCRATCH_SECINFO.FLAGS.R;
EPCM(DS:RCX).W := EPCM(DS:RCX).W & SCRATCH_SECINFO.FLAGS.W;
EPCM(DS:RCX).X := EPCM(DS:RCX).X & SCRATCH_SECINFO.FLAGS.X;

> This could be achieved e.g. by having ioctl to change protection
> bits in encl->page_tree.
> 
> This would simplify things a lot given that there would be only
> two, instead of three, EACCEPT code paths.

Reinette