Re: [PATCH V2 2/5] x86/sgx: Mark PCMD page as dirty when modifying contents

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Jarkko,

On 5/11/2022 3:43 AM, Jarkko Sakkinen wrote:
> On Mon, May 09, 2022 at 02:48:00PM -0700, Reinette Chatre wrote:
>> Recent commit 08999b2489b4 ("x86/sgx: Free backing memory
>> after faulting the enclave page") expanded __sgx_encl_eldu()
>> to clear an enclave page's PCMD (Paging Crypto MetaData)
>> from the PCMD page in the backing store after the enclave
>> page is restored to the enclave.
>>
>> Since the PCMD page in the backing store is modified the page
>> should be marked as dirty to ensure the modified data is retained.
>>
>> Fixes: 08999b2489b4 ("x86/sgx: Free backing memory after faulting the enclave page")
>> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
>> ---
>> Changes since RFC v1:
>> - Do not set dirty bit on enclave page since it is not being
>>   written to and always will be discarded.  (Dave)
>>
>>  arch/x86/kernel/cpu/sgx/encl.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
>> index 398695a20605..2521d64e8bcf 100644
>> --- a/arch/x86/kernel/cpu/sgx/encl.c
>> +++ b/arch/x86/kernel/cpu/sgx/encl.c
>> @@ -83,6 +83,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
>>  		ret = -EFAULT;
>>  	}
>>  
>> +	set_page_dirty(b.pcmd);
>>  	memset(pcmd_page + b.pcmd_offset, 0, sizeof(struct sgx_pcmd));
>>  
>>  	/*
>> -- 
>> 2.25.1
>>
> 
> LGTM.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Thank you very much.

Based on my understanding of Dave's feedback I do plan to move the
set_page_dirty() to be after the memset:
https://lore.kernel.org/linux-sgx/8922e48f-6646-c7cc-6393-7c78dcf23d23@intel.com/

Reinette