x86/sgx: Fix sgx_ioc_enclave_add_page() documentation
diff mbox series

Message ID 20190826053248.4403-1-jarkko.sakkinen@linux.intel.com
State New
Headers show
Series
  • x86/sgx: Fix sgx_ioc_enclave_add_page() documentation
Related show

Commit Message

Jarkko Sakkinen Aug. 26, 2019, 5:32 a.m. UTC
Refine the kdoc of sgx_ioc_enclave_add_page() with a proper description
about the interaction with mmap(). Without documentation, it is
impossible to review the code change.

Cc: Serge Ayoun <serge.ayoun@intel.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Sean Christoherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Jarkko Sakkinen Aug. 26, 2019, 5:59 a.m. UTC | #1
On Mon, Aug 26, 2019 at 08:32:48AM +0300, Jarkko Sakkinen wrote:
> Refine the kdoc of sgx_ioc_enclave_add_page() with a proper description
> about the interaction with mmap(). Without documentation, it is
> impossible to review the code change.
> 
> Cc: Serge Ayoun <serge.ayoun@intel.com>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: Sean Christoherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

I squashed this.

/Jarkko
Jarkko Sakkinen Aug. 26, 2019, 8:20 a.m. UTC | #2
On Mon, Aug 26, 2019 at 08:59:34AM +0300, Jarkko Sakkinen wrote:
> On Mon, Aug 26, 2019 at 08:32:48AM +0300, Jarkko Sakkinen wrote:
> > Refine the kdoc of sgx_ioc_enclave_add_page() with a proper description
> > about the interaction with mmap(). Without documentation, it is
> > impossible to review the code change.
> > 
> > Cc: Serge Ayoun <serge.ayoun@intel.com>
> > Cc: Casey Schaufler <casey@schaufler-ca.com>
> > Cc: Sean Christoherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> I squashed this.

Further defined the documentation to also address TCS:

/**
 * sgx_ioc_enclave_add_page() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGE
 * @filep:	open file to /dev/sgx
 * @arg:	a user pointer to a struct sgx_enclave_add_page instance
 *
 * Add (EADD) a page to an uninitialized enclave, and optionally extend
 * (EEXTEND) the measurement with the contents of the page. A SECINFO for a TCS
 * is required to always contain zero permissions because CPU silently zeros
 * them. Allowing anything else would cause a mismatch in the measurement.
 *
 * mmap()'s protection bits are capped by the page permissions. For each page
 * address, the maximum protection bits are computed with the following
 * heuristics:
 *
 * 1. A regular page: PROT_R, PROT_W and PROT_X match the SECINFO permissions.
 * 2. A TCS page: PROT_R | PROT_W.
 * 3. No page: PROT_NONE.
 *
 * mmap() is not allowed to surpass the minimum of the maximum protection bits
 * within the given address range.
 *
 * As stated above, a non-existent page is interpreted as a page with no
 * permissions. In effect, this allows mmap() with PROT_NONE to be used to seek
 * an address range for the enclave that can be then populated into SECS.
 *
 * Return:
 *   0 on success,
 *   -EINVAL if the SECINFO contains invalid data,
 *   -EACCES if the source page is located in a noexec partition,
 *   -ENOMEM if any memory allocation, including EPC, fails,
 *   -errno otherwise
 */

Saw no point to cycle this here other than mention about the change. I'd
say now it starts to be throughout enough.

/Jarkko

Patch
diff mbox series

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index ead9fb2d9b69..4d305b2c08e2 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -442,14 +442,22 @@  static int sgx_encl_add_page(struct sgx_encl *encl,
 }
 
 /**
- * sgx_ioc_enclave_add_page - handler for %SGX_IOC_ENCLAVE_ADD_PAGE
- *
+ * sgx_ioc_enclave_add_page() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGE
  * @filep:	open file to /dev/sgx
  * @arg:	a user pointer to a struct sgx_enclave_add_page instance
  *
  * Add a page to an uninitialized enclave (EADD), and optionally extend the
  * enclave's measurement with the contents of the page (EEXTEND).
  *
+ * SECINFO limits the maximum permissions, which can be given to mmap(). When
+ * mapping a range of pages, a page with least permissions will be the limit
+ * for the whole address range. This differing access levels to the enclave
+ * memory based on task privileges.
+ *
+ * A non-existent page is interpreted as a page with no permissions. In effect,
+ * this allows mmap() with PROT_NONE to be used to seek an address range for
+ * the enclave that can be then populated into SECS.
+ *
  * Return:
  *   0 on success,
  *   -EINVAL if other than RWX protection bits have been set