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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 17, 2023 at 05:03:05PM -0600, Haitao Huang wrote:
> Hi Jarkko
> 
> On Fri, 17 Feb 2023 16:32:12 -0600, jarkko@xxxxxxxxxx <jarkko@xxxxxxxxxx>
> wrote:
> 
> > On Wed, Feb 15, 2023 at 09:42:30AM -0600, Haitao Huang wrote:
> > > On Wed, 15 Feb 2023 02:51:23 -0600, Huang, Kai <kai.huang@xxxxxxxxx>
> > > 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.
> > 
> > Is this something you care in SGX SDK?
> > 
> > It is not categorically forbidden so that's why I'm asking.
> 
> SDK does not care as we would never do this and don't think anyone is doing
> that either. Suggesting returning error to cover all cases so user space
> would not accidentally cause incorrect vm_pgoff set.

vm_pgoff != 0 should result -EINVAL.

BR, Jarkko



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux