Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <59910ad4-a898-4eb2-5e2b-856c686b53fb@intel.com>
Date:   Tue, 5 Apr 2022 09:49:58 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.7.0
Subject: Re: [PATCH V3 14/30] x86/sgx: Support restricting of enclave page
 permissions
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>, <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>
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>,
        <nathaniel@profian.com>
References: <cover.1648847675.git.reinette.chatre@intel.com>
 <8ed9ee98ca26c9eefde0fd49062bca6e7b9efe80.1648847675.git.reinette.chatre@intel.com>
 <c9071aa19076c7c618294f0c9cb830a8be96ae09.camel@kernel.org>
 <26ab773de8842d03b40caf8645ca86884b195901.camel@kernel.org>
 <91a02d50b2cba34dfb058fd864ba20ef1e6a5554.camel@kernel.org>
 <10303ca73ea02a300636580e87446766374f66cb.camel@kernel.org>
 <6e0feeadc562b9e3f0a524040469d4f5c3484824.camel@kernel.org>
 <ca08465b6fa8af4121592c6381023fda5e0ade70.camel@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ca08465b6fa8af4121592c6381023fda5e0ade70.camel@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bDlQcFlYZU16WnFFY3lrYXQ4Q2EvYWxyckZiUnkvaHY5dHN2OStORWUwa3dE?=
 =?utf-8?B?bGo5ZTFyRVBRNW1PN2dvUURuajF4eTRRVllYZ1kyaGtGM3I1R1h2aVJZREhr?=
 =?utf-8?B?OFpMRnlBSHRmZXZsd0oxbmFGU21jWTdFYnJoelk5dDVmNGQ2MmhWUEkwSmRr?=
 =?utf-8?B?ZFIwNWtzbWJETHlsWVNSWWo0ckk3UFkvODQ4M3dtVTRRdnFhZEsySEF3U2JU?=
 =?utf-8?B?ZXJod0ltemZUQWROcE9CTkRCVjF0R092TWRkekxycCszVlo2bzFma1puaE9N?=
 =?utf-8?B?bzJJbjI2QjhNRW9Vc2VEUGJtSTFvVGRXU0d5VS9WV3I0SmgzQU9kZHhPVUJm?=
 =?utf-8?B?M25qQUdVQ1pMdUJwV3IvVWY3K2pKdnVFdHpVazAzR3JyajM5UGtQRk1tS2Nl?=
 =?utf-8?B?ajZnekFMdng3aVBxNU9IekdwWGFkY0ZSVDNQck1jNlROWHBERVZHN2c0anA4?=
 =?utf-8?B?MCsxWi9PK0FHeUJuVFdUeGdEY1liNUVuZmo5eURBZGpzeTJzQlBtV0ZnZmgr?=
 =?utf-8?B?bm9nTFRXTW9xajVrNFpnNkdMTGdidG05WGpzTlVXSy85SjNiTkI0YVNBMEY3?=
 =?utf-8?B?bTFXSjRJemZNbUhMSGtsSDBkWjlIUmdHTmtoclZpTWlmd2xZM2dnK2J1MGdU?=
 =?utf-8?B?ZzFkMk1zNCtCcDdoNHJ2Q0Zqb2ExMnM0aUFCZG12M0J5QkVYYS9aemxzTXU3?=
 =?utf-8?B?ZU5Cc2NQM2FodXNNVXJUUGx1UmtQN2IzTGNsOUx0cDBqcUJTVlh3Qm9MUEZj?=
 =?utf-8?B?NWNncVNiOFdFTXJHUVZFYjFZWmRZOHZuaWlpazNZc1dkbDlrcFJhSlc2VDFJ?=
 =?utf-8?B?YlBuckJxVFNPVVpIVEV6LzNUV3M4RktJbDUwZTZEOFN2L0h1VmljMTBoZTNh?=
 =?utf-8?B?MnNKZ1lsS1pTeVNOOG9jMm96UzB5WEd4YTBnRWU3bTVDaU5URE4rSUZsWW9Y?=
 =?utf-8?B?SkY1MFRzdzNvV1crSmNhTi96dG9zSHIwVU9LTC9BT2VIRFNUUENsZkFqSmda?=
 =?utf-8?B?YmQ5Y0FiNzhZL3FDNFR0OTZveFBiRlBya3lVanBqY3BmTVlReFE1ZFZ4WFhj?=
 =?utf-8?B?TndCOTE2S0F4eFNmZjZjSVVNMGtVR01xcTZsSjhoT0FVRkdoMjFsWVBCd1V4?=
 =?utf-8?B?VzRLbmJJR1NMZzVmKzdLUVpZb0NpQTNqWGJLdDlRT0ZpaURXbnBDQkZ5TVln?=
 =?utf-8?B?T1V6M0ZJUlNTUjlQd0QrZTc4YlIxUERaZG5oaTNwV2JZY1RIY1NQaW4wbFAy?=
 =?utf-8?B?NG4xQUppTGlpS2dMVVBnaWFnQVlZcWR3Y0U3T0dVQTA0eStnK3ZqLzZ5OUY0?=
 =?utf-8?B?TFprRTU2M2p1Y240NG1hdkVJQ1dObm5laHFwQ1ExcU9sOHV6MDNLN3p0M1Bn?=
 =?utf-8?B?RXI2bHBIYXRSQzFxUGJ1STlwWFhJcHdKSTZjUkdZUVhyMVFFMWlTenR5TXVh?=
 =?utf-8?B?U1lGbGlMR1ViTjFITjNaNmZ6aGxPTDZxdGhQc2Rzd25WblBNbmZQYm5Yd3JU?=
 =?utf-8?B?bUE5V0UwdHYyeVNmYndaM3VBc3hIb1UwV1k3eThlMDB5dndWQ1NHaEEyQUo3?=
 =?utf-8?B?UkFNdnJiejRGdC9LRVY2UGVNWVNIU0JzV1gra1owa292L05SVWNsVWttaUhV?=
 =?utf-8?B?OXlqMXovMGltZ2hpQXIwU1NJZkNob3ZYZktIRTZTZThmT2lLOW1WMGZKbkxU?=
 =?utf-8?B?L09PempGSHNXNkJhRUNCT0IwbUYvam4xelIwZjdmbElNWTFYMXFhUjd1TDBk?=
 =?utf-8?B?aVJnNEhsZUovL0JyY2UweVVWdlNWa2c0L1JGZjQ2b2VIaVhxYmFqd3NpV1o1?=
 =?utf-8?B?VVlUZm5BbUNic1dVQmlxc0JSMDlPZXhEYnFvdjVua1lCQ1g4RzJGa0MveHVp?=
 =?utf-8?B?TU1xSXFwMWcyRUtZMkkyamJhcUZNUGVKOURTeVVXZmZod3UzY1B6eUl0VmlX?=
 =?utf-8?B?SGxjQnUvczZSNmZRZUt4UlN3NUI2REVZZklPbFZ6WEZodzA5bFcyZEY5ekhQ?=
 =?utf-8?B?b1hFSy91WmJHeUNoOHl3MmlocGd4VDdOVmR0VndvaFpubVd5U2lFQUFkM0lr?=
 =?utf-8?B?TGtBc3pwMEpDbWV0Q0NYTUlEckk0TEVOQ1NDeDFuZWUxZ1N0alVCVDNKc205?=
 =?utf-8?B?NlpiNUxZYkM4ZStnZEpwMHM5RUIreFNNR2x5YnUwYkw0OW11Zm4relhVS3Rt?=
 =?utf-8?B?Sy9WUlk4eFBsZCtTT3RTbnR3TW5teVdPd1ZraUFtc2lBNllOeTlYZ0swTi9a?=
 =?utf-8?B?Sjg3MHM2MDR1aXlRRUlWSDFwMlMydjdYT1QwdkYrcmpkRTlXWHhTNWJzYmFQ?=
 =?utf-8?B?dVNNaEdxMmhFejdMeWpLS0VvMnpsYVFvU3M5M3JlMVowdmI2ZjhUc0pFd3Ex?=
 =?utf-8?Q?WAqPOws5ibRQ/3PE=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1a3be220-c7ca-4f56-3007-08da1724545b
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2022 16:50:03.0369
 (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: gt43u5fEDtnyKByZ7PtuWujQABqML0Md6uoixMzX8H8+W1w5CCWx8YL0enbfDhP77iqHEgnOJbTCVc6FYAxISCtqpSF/rZRtwjsrfuj39Fo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5164
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jarkko,

On 4/5/2022 7:52 AM, Jarkko Sakkinen wrote:
> n Tue, 2022-04-05 at 17:27 +0300, Jarkko Sakkinen wrote:
>> According to SDM having page type as regular is fine for EMODPR,
>> i.e. that's why I did not care about having it in SECINFO.
>>
>> Given that the opcode itself contains validation, I wonder
>> why this needs to be done:
>>
>> if (secinfo.flags & ~SGX_SECINFO_PERMISSION_MASK)
>>                 return -EINVAL;
>>
>> if (memchr_inv(secinfo.reserved, 0, sizeof(secinfo.reserved)))
>>                 return -EINVAL;
>>
>> perm = secinfo.flags & SGX_SECINFO_PERMISSION_MASK;
>>
>> I.e. why duplicate validation and why does it have different
>> invariant than the opcode?
> 
> Right it is done to prevent exceptions and also pseudo-code
> has this validation:
> 
> IF (EPCM(DS:RCX).PT is not PT_REG) THEN #PF(DS:RCX); FI; 

The current type of the page is validated - not the page type
provided in the parameters of the command.

> 
> This is clearly wrong:

Could you please elaborate what is wrong? The hardware only checks
the permission bits and that is what is provided.

> 
> /*
>  * Return valid permission fields from a secinfo structure provided by
>  * user space. The secinfo structure is required to only have bits in
>  * the permission fields set.
>  */
> static int sgx_perm_from_user_secinfo(void __user *_secinfo, u64 *secinfo_perm)
> 
> It means that the API requires a malformed data as input.

It is not clear to me how this is malformed. The API requires that only
the permission bits are set in the secinfo, only the permission bits in secinfo
is provided to the hardware, and the hardware only checks the permission bits.

> 
> Maybe it would be better idea then to replace secinfo with just the
> permission field?

That is what I implemented in V1 [1], but was asked to change to secinfo. I could
go back to that if you prefer.

Reinette

[1] https://lore.kernel.org/linux-sgx/44fe170cfd855760857660b9f56cae8c4747cc15.1638381245.git.reinette.chatre@intel.com/