Re: [PATCH v29 00/20] Intel SGX foundations

[Jarkko Sakkinen]

On Thu, May 28, 2020 at 02:15:18PM +0300, Jarkko Sakkinen wrote:
> On Thu, May 07, 2020 at 05:25:55PM -0700, Sean Christopherson wrote:
> > Ah, fudge.  shmem_zero_setup() triggers shmem_acct_size() and thus
> > __vm_enough_memory().  Which I should have rememered because I've stared
> > at that code several times when dealing with the enclave's backing store.
> > I wasn't seeing the issue because I happened to use MAP_PRIVATE.
> > 
> > So, bad analysis, good conclusion, i.e. the kernel is still doing the
> > right thing, it's just not ideal for userspace.
> > 
> > 
> > Jarkko, we should update the docs and selftest to recommend and use
> > 
> >   PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS
> > 
> > or
> > 
> >   PROT_NONE, MAP_SHARED | MAP_NORESERVE | MAP_ANONYMOUS"
> > 
> > when carving out ELRANGE, with an explicit comment that all the normal
> > rules for mapping memory still apply.
> 
> Ugh, had forgotten this.
> 
> OK, I guess this comment explains it all:
> 
> "
> /*
>  * shmem_file_setup pre-accounts the whole fixed size of a VM object,
>  * for shared memory and for shared anonymous (/dev/zero) mappings
>  * (unless MAP_NORESERVE and sysctl_overcommit_memory <= 1),
>  * consistent with the pre-accounting of private mappings ...
>  */
> static inline int shmem_acct_size(unsigned long flags, loff_t size)
> "

Do not agree though that any documentation should be produced but the
selftest should have correct parameters, yes. Instructions on how to
reserve a range of addresses simply does not belong to SGX documentation
because it is not SGX related in the first place. The patterns you
showed are universal.

I'll fix just the selftest for v31.

/Jarkko