* [PATCH v2] x86/sgx: Allows ioctl PROVISION to execute before CREATE
@ 2021-01-20 3:53 Tianjia Zhang
2021-01-20 16:20 ` Sean Christopherson
0 siblings, 1 reply; 2+ messages in thread
From: Tianjia Zhang @ 2021-01-20 3:53 UTC (permalink / raw)
To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, Sean Christopherson, x86, linux-sgx,
linux-kernel, Jia Zhang
Cc: Tianjia Zhang
In function sgx_encl_create(), the logic of directly assigning
value to attributes_mask determines that the call to
SGX_IOC_ENCLAVE_PROVISION must be after the command of
SGX_IOC_ENCLAVE_CREATE. If move this assignment statement to
function sgx_open(), the PROVISION command can be executed
earlier and more flexibly.
Reported-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
arch/x86/kernel/cpu/sgx/driver.c | 3 +++
arch/x86/kernel/cpu/sgx/ioctl.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index f2eac41bb4ff..8766580194ae 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -36,6 +36,9 @@ static int sgx_open(struct inode *inode, struct file *file)
return ret;
}
+ encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT |
+ SGX_ATTR_KSS;
+
file->private_data = encl;
return 0;
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 90a5caf76939..1c6ecf9fbeff 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
encl->base = secs->base;
encl->size = secs->size;
encl->attributes = secs->attributes;
- encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS;
/* Set only after completion, as encl->lock has not been taken. */
set_bit(SGX_ENCL_CREATED, &encl->flags);
--
2.19.1.3.ge56e4f7
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] x86/sgx: Allows ioctl PROVISION to execute before CREATE
2021-01-20 3:53 [PATCH v2] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
@ 2021-01-20 16:20 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2021-01-20 16:20 UTC (permalink / raw)
To: Tianjia Zhang
Cc: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, x86, linux-sgx, linux-kernel, Jia Zhang
On Wed, Jan 20, 2021, Tianjia Zhang wrote:
> In function sgx_encl_create(), the logic of directly assigning
> value to attributes_mask determines that the call to
> SGX_IOC_ENCLAVE_PROVISION must be after the command of
> SGX_IOC_ENCLAVE_CREATE. If move this assignment statement to
> function sgx_open(), the PROVISION command can be executed
> earlier and more flexibly.
Use imperative mood to describe what you're doing, e.g. the "if" leaves the
reader wonder whether it's a suggestion or what the patch is actually doing.
E.g. something like
Move the initialization of an enclave's allowed attributes_mask to
sgx_open() to allow the ENCLAVE_PROVISION ioctl() to be invoked before
ENCLAVE_CREATE.
> Reported-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
> arch/x86/kernel/cpu/sgx/driver.c | 3 +++
> arch/x86/kernel/cpu/sgx/ioctl.c | 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
> index f2eac41bb4ff..8766580194ae 100644
> --- a/arch/x86/kernel/cpu/sgx/driver.c
> +++ b/arch/x86/kernel/cpu/sgx/driver.c
> @@ -36,6 +36,9 @@ static int sgx_open(struct inode *inode, struct file *file)
> return ret;
> }
>
> + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT |
> + SGX_ATTR_KSS;
Probably don't need a newline before SGX_ATTR_KSS. In fact, I wouldn't be
surprised if Boris explicitly requested letting the original code go past 80
chars to improve readability. Either way, with a cleaned up changelog:
Reviewed-by: Sean Christopherson <seanjc@google.com>
> +
> file->private_data = encl;
>
> return 0;
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 90a5caf76939..1c6ecf9fbeff 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
> encl->base = secs->base;
> encl->size = secs->size;
> encl->attributes = secs->attributes;
> - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS;
>
> /* Set only after completion, as encl->lock has not been taken. */
> set_bit(SGX_ENCL_CREATED, &encl->flags);
> --
> 2.19.1.3.ge56e4f7
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-20 16:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 3:53 [PATCH v2] x86/sgx: Allows ioctl PROVISION to execute before CREATE Tianjia Zhang
2021-01-20 16:20 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).