[for_v23,v3,00/12] x86/sgx: Bug fixes for v23

Message

Sean Christopherson Oct. 16, 2019, 6:37 p.m. UTC

As requested, the merge of the EPC and EADD bug fix series.  The headliners
are two critical bug fixes for a memory leak in sgx_encl_destroy() and a
livelock due to the EPC page free count getting corrupted.

Sean Christopherson (12):
  x86/sgx: Pass EADD the kernel's virtual address for the source page
  x86/sgx: Check the validity of the source page address for EADD
  x86/sgx: Fix EEXTEND error handling
  x86/sgx: Drop mmap_sem before EEXTENDing an enclave page
  x86/sgx: Remove redundant message from WARN on non-emtpy mm_list
  x86/sgx: Fix a memory leak in sgx_encl_destroy()
  x86/sgx: WARN on any non-zero return from __eremove()
  x86/sgx: WARN only once if EREMOVE fails
  x86/sgx: Split second half of sgx_free_page() to a separate helper
  x86/sgx: Use the post-reclaim variant of __sgx_free_page()
  x86/sgx: Don't update free page count if EPC section allocation fails
  x86/sgx: Reinstate per EPC section free page counts

 arch/x86/kernel/cpu/sgx/encl.c    | 29 ++++++++++-------
 arch/x86/kernel/cpu/sgx/ioctl.c   | 52 +++++++++++++++--------------
 arch/x86/kernel/cpu/sgx/main.c    | 54 ++++++++++++++++++++++---------
 arch/x86/kernel/cpu/sgx/reclaim.c |  8 ++---
 arch/x86/kernel/cpu/sgx/sgx.h     | 19 ++++++++++-
 5 files changed, 106 insertions(+), 56 deletions(-)

Comments

Jarkko Sakkinen Oct. 17, 2019, 6:10 p.m. UTC | #1

On Wed, Oct 16, 2019 at 11:37:33AM -0700, Sean Christopherson wrote:
> As requested, the merge of the EPC and EADD bug fix series.  The headliners
> are two critical bug fixes for a memory leak in sgx_encl_destroy() and a
> livelock due to the EPC page free count getting corrupted.

BTW, are you still planning to send multi-page EADD? I think it should
be done. It is obviously superior design when combined with my small
twist where mrmask is calculated inside the kernel.

Not only does it provide multi-page EADD but you could always do one
ioctl per area. With mrmask in parameters you end up needing two if the
user space is designed in a way that measurement is not a multiple of a
page.

If mrmask would make sense in the parameters, then a single-page EADD
would be cool but I think we want to render it out completely.

/Jarkko

Jarkko Sakkinen Oct. 17, 2019, 6:12 p.m. UTC | #2

On Thu, Oct 17, 2019 at 09:10:54PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 11:37:33AM -0700, Sean Christopherson wrote:
> > As requested, the merge of the EPC and EADD bug fix series.  The headliners
> > are two critical bug fixes for a memory leak in sgx_encl_destroy() and a
> > livelock due to the EPC page free count getting corrupted.
> 
> BTW, are you still planning to send multi-page EADD? I think it should
> be done. It is obviously superior design when combined with my small
> twist where mrmask is calculated inside the kernel.
> 
> Not only does it provide multi-page EADD but you could always do one
> ioctl per area. With mrmask in parameters you end up needing two if the
> user space is designed in a way that measurement is not a multiple of a
> page.
> 
> If mrmask would make sense in the parameters, then a single-page EADD
> would be cool but I think we want to render it out completely.

My earlier comments about doing first a single page EADD were based on
API where you provide mrmask.

/Jarkko

Jarkko Sakkinen Oct. 18, 2019, 1:13 p.m. UTC | #3

On Thu, Oct 17, 2019 at 09:10:54PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 11:37:33AM -0700, Sean Christopherson wrote:
> > As requested, the merge of the EPC and EADD bug fix series.  The headliners
> > are two critical bug fixes for a memory leak in sgx_encl_destroy() and a
> > livelock due to the EPC page free count getting corrupted.
> 
> BTW, are you still planning to send multi-page EADD? I think it should
> be done. It is obviously superior design when combined with my small
> twist where mrmask is calculated inside the kernel.
> 
> Not only does it provide multi-page EADD but you could always do one
> ioctl per area. With mrmask in parameters you end up needing two if the
> user space is designed in a way that measurement is not a multiple of a
> page.
> 
> If mrmask would make sense in the parameters, then a single-page EADD
> would be cool but I think we want to render it out completely.

I applied the fixes that I saw were appropriate. You can check if
this has been sufficient and send a revised set of fixes if not.

/Jarkko