Re: [RFC PATCH v4 2/4] x86/sgx: Implement support for MADV_WILLNEED

Haitao Huang
2023-02-15T15:43:04Z

On Wed, 15 Feb 2023 02:51:23 -0600, Huang, Kai <kai.huang@intel.com> wrote:

>> > > >
>> >
>> > Since sgx_mmap() can happen before enclave is created, calculating the
>> > vm_pgoff
>> > from enclave_base is conceptually wrong. Even if you really want to
>> do
>> > it, it
>> > should be:
>> >
>> > if (enclave_has_initialized())
>> > vma->vm_pgoff = ...;
>>
>> I got your point now. I can add a condition to test the SGX_ENCL_CREATED
>> bit. However, we still have a hole if we must handle the sequence
>> mmap(..., enclave_fd) being called before ECREATE ioctl. We can't leave
>> vm_pgoff not set for those cases.
>>
>> Since no one does that so far, can we explicitly return an error from
>> sgx_mmap when that happens?
>> Other suggestions?
>
> As I replied to patch 4/4, I believe userspace should pass the correct
> pgoff in
> mmap(). It's wrong to always pass 0 or any random value.
>
> If userspace follow the mmap() rule, you won't need to manually set
> vm_pgoff
> here (which is hacky IMHO). Everything works fine.
>

SGX driver was following MAP_ANONYMOUS semantics. If we change that, it'd
break current usage/ABI.

I still think returning error for cases mmap(..., enclave_fd) if enclave
is not created would be less intrusive change.

Haitao