On Mon, 2019-06-17 at 15:24 -0700, Sean Christopherson wrote:
> The enclave mm tracking is currently broken:
>
> - Adding current->mm during ECREATE is wrong as there is no guarantee
> that the current process has mmap()'d the enclave, i.e. there may
> never be an associated sgx_vma_close() to drop the encl_mm.
>
> - Adding mm's at sgx_vma_open() is wrong as vm_ops->open is called
> only when splitting or duplicating a vma. If userspace performs a
> single mmap() on the enclave then SGX will fail to track the mm.
> This bug is partially hidden by tracking current->mm at ECREATE.
>
> Rework the tracking to get/add the mm at mmap(). A side effect of the
> bug fix is that sgx_vma_{open,close}() should never encounter a vma with
> an associated enclave and no associated encl_mm, i.e. WARN if an encl_mm
> cannot be found in either condition.
>
> Change the WARN() on a non-empty mm_list to a WARN_ONCE(). The warning
> will fire over and over (and over) if the mm tracking is broken, which
> hampers debug/triage far more than it helps.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> arch/x86/kernel/cpu/sgx/driver/ioctl.c | 14 ----------
> arch/x86/kernel/cpu/sgx/driver/main.c | 26 ++++++++++++++++++
> arch/x86/kernel/cpu/sgx/encl.c | 38 +++++---------------------
> arch/x86/kernel/cpu/sgx/encl.h | 4 +--
> 4 files changed, 35 insertions(+), 47 deletions(-)
BTW, sgx_encl_mm is a bit confusing name (which I made) to begin with.
It clues as it was a some kind of 1:1 association with the process,
which it isn't.
Could you update the patch and rename it as sgx_encl_mapping? After that
I'm happy to merge.
/Jarkko
