On Wed, 2019-06-19 at 15:56 +0300, Jarkko Sakkinen wrote:
> 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.
And send it as a separate patch. Can merge it today/tomorrow after I've
finished my reaper change.
/Jarkko
