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: <76c6e673-71fb-1068-0114-c3eea93a2fd4@intel.com>
Date:   Tue, 5 Apr 2022 10:21:22 -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 RFC] x86/sgx: Simplify struct
 sgx_enclave_restrict_permissions
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>, <linux-sgx@vger.kernel.rog>
CC:     Dave Hansen <dave.hansen@linux.intel.com>,
        Nathaniel McCallum <nathaniel@profian.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "open list:INTEL SGX" <linux-sgx@vger.kernel.org>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
References: <20220405151642.96096-1-jarkko@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20220405151642.96096-1-jarkko@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UTNudGJQQWhoRHlncEtib3VMSE84dHZBaWxBbjV5dXNRWkd4RVFqeHRqc0NG?=
 =?utf-8?B?eTliQXV2d3liZXpqNGVzWmNTOExySWRQcytGbDdzWkxmL2hhYXRsdUxHVFhz?=
 =?utf-8?B?ZkpGdkN2RUVhQVUxZGoyazBiQjU0YUtneXJnbDhyMHJYclk1Y3NXZXBVMGlD?=
 =?utf-8?B?bURzbjNPRm12dklHc1ZSYytpdzBldDcyYS9zZzBBNWZBWDMrUVVkL3hRVmh0?=
 =?utf-8?B?SS9NbWd4RDhRNVVZTDNEVW9zY2x2dWZ6eDFjRUZKanFaK2toaEdLVCtaYk5i?=
 =?utf-8?B?TGlDdWU3NkRpTCtxMGtKU2x3TWdFdDJ3TmdGakJNMFJ5TVVuZUMzWmtJdDF0?=
 =?utf-8?B?Q1laUnRKSUhTYUt2QXZXbWZUN0pHRlNJWVdaMkNUa0ErSmlYMHZ3SDAwTitF?=
 =?utf-8?B?OW9RSHlBM2VWWCtESXF0ZytnZDVkZjJSd0hucDl1alNoemRSZU5UcGZqdUwr?=
 =?utf-8?B?UXpGeTVmdGVwSkpvYXVHMWlwQitxeHp0N29nbENXV3FBMGRBSmtsWDdDUjBx?=
 =?utf-8?B?WitSSWhPSld2dmNNMWNQZE81UitFOUkvSGZtN09vVVJOSFJqLzI2c09kVDh4?=
 =?utf-8?B?d0crNjJDNVVSK1pUMWorTVNpSlQxOFoxRFExZkdsMS9aMDdPQzM0NW01VXBs?=
 =?utf-8?B?YmRiaHBINGdTWGdubVdyUXVzZVpBM0VjTnpCbTdlNUV3ZGVJcWhZMWt3RG9E?=
 =?utf-8?B?dkJBR2pCNkp0S2ZRWThudURoZG1TcHlxbUJhdkpPK1JHc25BSXNXNmxMOGd1?=
 =?utf-8?B?Rys5UThRNkI0MnA3WVRhUUFhUFByR2VpSStQRzBqeG9VMjNDTGtLN3VzMXM4?=
 =?utf-8?B?TUxtUGx0UXJzaTc5R29Ha3RwZE1Fd2tVOHZwcnRFMG9YOFdmVjhLZi9OdVRz?=
 =?utf-8?B?dnRFMnN1aUlkUkVtT0hzNWR4dHM0RGxHb01PdEZ2cGJZclg2VHZWMUVxdWNW?=
 =?utf-8?B?d29kMUVSb3drNjdGUjc2WlNYMkxrT1BCQXM3ckhYekNKT1huTkFNWUwvWmhR?=
 =?utf-8?B?NEluSDI4UE1XRDgyOHhHamxueE9XbC9XQWtFYmxNR1g5eG5MQVB1Z0hUTlRC?=
 =?utf-8?B?RjE1a3l4Y0htMnNTSWtOVWlDVUo3Q2cyTjdLZi90MkRMSUM2eVFUZU42ZzBp?=
 =?utf-8?B?ZlpOL1ZrRjVJMVN6c2grMzVuRXdXOFVTUFdCdkZ2OXBEMlVYTkVkVWJ1NVFz?=
 =?utf-8?B?UVJEWEtnNDZvd3ExaVZQREVLRkxxSXh4TWpwdmlOdEdUZkhzR3liaGlyNDh0?=
 =?utf-8?B?bVhWcmhpZHpKZytYaUJPWVBxam8rdENIQTM5RExUMW15RGc1U2JDYXF0UldU?=
 =?utf-8?B?OWsyeTNjNU15Q29aUEFnV0xrMGg2ZVlJQjJ5M1JPR2RES1ozRVV2Sm1neW53?=
 =?utf-8?B?WCtPbU93WU5oMDM2L0JXN1Ruam1UMXRLMW1XZ3JETHhaSTJ3MWxGYjhsbEtG?=
 =?utf-8?B?ZS9FSVBlZ0t2SzZ4L2ZoS25wNzVOaExNaU9PbWJnTzV6SHlINzFyZGFJT2c4?=
 =?utf-8?B?K0pCRk1BUWZQVm5TQjZGQVZEZWhSYnJIL241eldJOE0yb0lUZno3U2pZL0M3?=
 =?utf-8?B?K0NiMkdCcGNrU0U3ZC93TWcxN1ZSZTRQRyt5bzRuMGxKaFV3VVgxcDQ3RWZH?=
 =?utf-8?B?SXlZeDB0SGcvaldTMjlRdTlKRE4wRitkb2prT011M2dDVXMvWmxsMkZEYWxu?=
 =?utf-8?B?OU91dUVJV2lzeEVTUEd1ZjFEdS9peERqZnBrTmtQWU1iNEU2UnlvSUNXUEJq?=
 =?utf-8?B?Rjg2OHFza2JnY0JaWFNsd3ZRMm15N0lwbytXSGtHbWRZT2hRZHpwK1RLek9C?=
 =?utf-8?B?OEt2b2pwTy92K3RnaVQ5VjhjdEZvVGRRNitocEZNZTU0UU5uWDF0cEpyU3NQ?=
 =?utf-8?B?NkVXdWpoREV5bDFXSTVBclRqNWJLejZNOGFnQjJvTnVFNzhlWEVPZGUvbUtV?=
 =?utf-8?B?bWhCdTNoaG1zdUxaTGI2dEt0aTJWdUJ2QmEzWVE5Rkt3SnBVUHIzN1BXcFpG?=
 =?utf-8?B?ZndQK203SC9NaC9zS0JTdkRmNW15aGsyWHdlc1lqYk1ORnBGNCt0cXlGNm1p?=
 =?utf-8?B?cUZSY1J2RkZqVlJvU1NMdWVraEtnN21MYklUTG1NVFc0cFZlVFF0b2YxT1h6?=
 =?utf-8?B?RmYrbE9ySkRDZGVwbGVwNlI3UCtlMi9BMEVnSC9lVzhRVEs4ei9taHRhR0hW?=
 =?utf-8?B?VERwZStNMXkxSVF5ays5c2xmN29UcTBkc1dzbjM0TTdHZVR2bnhxVmxiQjU1?=
 =?utf-8?B?V3ZYRVovMVNFdXZWUUJNekFnU0owNUh0ZDlQMkM0QTZ0eEk2RzNWa00rVExj?=
 =?utf-8?B?WlpjUWFYUjR3cGdFVGxyY2NXOVRiNERpT0xOWFFSOXZNd1h6TXpqKzV3ckgy?=
 =?utf-8?Q?JcoEJShNIILCSlNg=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 749e4ec5-36d6-44ea-c204-08da1728b7a4
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2022 17:21:27.5379
 (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: KKRBeoT3k9sultEJ+2GkaYotQ9CODgSiq0sYUCfhZVvceum00tleo7fxQX7+l4sybhrG2pItgSodYyq1GJTPgM0kakuBeLl2fGka25OcJm4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1706
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.6 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 8:16 AM, Jarkko Sakkinen wrote:
> The reasoning to change SECINFO to simply flags is stated in this inline
> comment:
> 
> /*
>  * 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.
>  */
> 
> It is better to simply change the parameter type than require to use
> a malformed version of a data structure.

Could you please elaborate what is malformed?

> 
> Link: https://lore.kernel.org/linux-sgx/26ab773de8842d03b40caf8645ca86884b195901.camel@kernel.org/T/#u
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> Only compile-tested.
>  arch/x86/include/uapi/asm/sgx.h |  5 ++-
>  arch/x86/kernel/cpu/sgx/ioctl.c | 57 ++++++---------------------------
>  2 files changed, 12 insertions(+), 50 deletions(-)
> 
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index feda7f85b2ce..627136798f2a 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -88,15 +88,14 @@ struct sgx_enclave_provision {
>   * @offset:	starting page offset (page aligned relative to enclave base
>   *		address defined in SECS)
>   * @length:	length of memory (multiple of the page size)
> - * @secinfo:	address for the SECINFO data containing the new permission bits
> - *		for pages in range described by @offset and @length
> + * @flags:	flags field of the SECINFO data
>   * @result:	(output) SGX result code of ENCLS[EMODPR] function
>   * @count:	(output) bytes successfully changed (multiple of page size)
>   */
>  struct sgx_enclave_restrict_permissions {
>  	__u64 offset;
>  	__u64 length;
> -	__u64 secinfo;
> +	__u64 flags;
>  	__u64 result;
>  	__u64 count;
>  };

What is the motivation for using the flags field of the SECINFO data? If
the goal is to only provide necessary information, why not just provide the
permission bits since none of the other bits are used? If the goal is
to make room for future SECINFO changes/demands, why not include the
reserved field of SECINFO where future changes are most likely to reside? 

Reinette