| Message ID | 20e09daf559aa5e9e680a0b4b5fba940f1bad86e.1616136308.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM SGX virtualization support | expand |
On Fri, Mar 19, 2021 at 08:23:08PM +1300, Kai Huang wrote: > + /* > + * @secs is an untrusted, userspace-provided address. It comes from > + * KVM and is assumed to be a valid pointer which points somewhere in > + * userspace. This can fault and call SGX or other fault handlers when > + * userspace mapping @secs doesn't exist. > + * > + * Add a WARN() to make sure @secs is already valid userspace pointer > + * from caller (KVM), who should already have handled invalid pointer > + * case (for instance, made by malicious guest). All other checks, > + * such as alignment of @secs, are deferred to ENCLS itself. > + */ > + WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)); So why do we continue here then? IOW: diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c index fdfc21263a95..497b06fc6f7f 100644 --- a/arch/x86/kernel/cpu/sgx/virt.c +++ b/arch/x86/kernel/cpu/sgx/virt.c @@ -270,7 +270,7 @@ int __init sgx_vepc_init(void) * * Return: * - 0: ECREATE was successful. - * - -EFAULT: ECREATE returned error. + * - <0: ECREATE returned error. */ int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, int *trapnr) @@ -288,7 +288,9 @@ int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, * case (for instance, made by malicious guest). All other checks, * such as alignment of @secs, are deferred to ENCLS itself. */ - WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)); + if (WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE))) + return -EINVAL; + __uaccess_begin(); ret = __ecreate(pageinfo, (void *)secs); __uaccess_end(); > + __uaccess_begin(); > + ret = __ecreate(pageinfo, (void *)secs); > + __uaccess_end(); > + > + if (encls_faulted(ret)) { > + *trapnr = ENCLS_TRAPNR(ret); > + return -EFAULT; > + } > + > + /* ECREATE doesn't return an error code, it faults or succeeds. */ > + WARN_ON_ONCE(ret); > + return 0; > +} > +EXPORT_SYMBOL_GPL(sgx_virt_ecreate); > + > +static int __sgx_virt_einit(void __user *sigstruct, void __user *token, > + void __user *secs) > +{ > + int ret; > + > + /* > + * Make sure all userspace pointers from caller (KVM) are valid. > + * All other checks deferred to ENCLS itself. Also see comment > + * for @secs in sgx_virt_ecreate(). > + */ > +#define SGX_EINITTOKEN_SIZE 304 > + WARN_ON_ONCE(!access_ok(sigstruct, sizeof(struct sgx_sigstruct)) || > + !access_ok(token, SGX_EINITTOKEN_SIZE) || > + !access_ok(secs, PAGE_SIZE)); Ditto.
On Mon, 5 Apr 2021 11:07:59 +0200 Borislav Petkov wrote: > On Fri, Mar 19, 2021 at 08:23:08PM +1300, Kai Huang wrote: > > + /* > > + * @secs is an untrusted, userspace-provided address. It comes from > > + * KVM and is assumed to be a valid pointer which points somewhere in > > + * userspace. This can fault and call SGX or other fault handlers when > > + * userspace mapping @secs doesn't exist. > > + * > > + * Add a WARN() to make sure @secs is already valid userspace pointer > > + * from caller (KVM), who should already have handled invalid pointer > > + * case (for instance, made by malicious guest). All other checks, > > + * such as alignment of @secs, are deferred to ENCLS itself. > > + */ > > + WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)); > > So why do we continue here then? IOW: The intention was to catch KVM bug, since KVM is the only caller, and in current implementation KVM won't call this function if @secs is not a valid userspace pointer. But yes we can also return here, but in this case an exception number must also be specified to *trapnr so that KVM can inject to guest. It's not that straightforward to decide which exception should we inject, but I think #GP should be OK. Please see below. > > diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c > index fdfc21263a95..497b06fc6f7f 100644 > --- a/arch/x86/kernel/cpu/sgx/virt.c > +++ b/arch/x86/kernel/cpu/sgx/virt.c > @@ -270,7 +270,7 @@ int __init sgx_vepc_init(void) > * > * Return: > * - 0: ECREATE was successful. > - * - -EFAULT: ECREATE returned error. > + * - <0: ECREATE returned error. > */ > int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, > int *trapnr) > @@ -288,7 +288,9 @@ int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, > * case (for instance, made by malicious guest). All other checks, > * such as alignment of @secs, are deferred to ENCLS itself. > */ > - WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)); > + if (WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE))) > + return -EINVAL; > + *trapnr should also be set to an exception before return -EINVAL. It's not possible to get it from ENCLS_TRAPNR(ret) like below, since ENCLS hasn't been run yet. I think it makes sense to just set it to #GP(X86_TRAP_GP), since above error basically means SECS is not pointing to an valid EPC address, and in such case #GP should happen based on SDM (SDM 40.3 ECREATE). > __uaccess_begin(); > ret = __ecreate(pageinfo, (void *)secs); > __uaccess_end(); > > > + __uaccess_begin(); > > + ret = __ecreate(pageinfo, (void *)secs); > > + __uaccess_end(); > > + > > + if (encls_faulted(ret)) { > > + *trapnr = ENCLS_TRAPNR(ret); > > + return -EFAULT; > > + } > > + > > + /* ECREATE doesn't return an error code, it faults or succeeds. */ > > + WARN_ON_ONCE(ret); > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(sgx_virt_ecreate); > > + > > +static int __sgx_virt_einit(void __user *sigstruct, void __user *token, > > + void __user *secs) > > +{ > > + int ret; > > + > > + /* > > + * Make sure all userspace pointers from caller (KVM) are valid. > > + * All other checks deferred to ENCLS itself. Also see comment > > + * for @secs in sgx_virt_ecreate(). > > + */ > > +#define SGX_EINITTOKEN_SIZE 304 > > + WARN_ON_ONCE(!access_ok(sigstruct, sizeof(struct sgx_sigstruct)) || > > + !access_ok(token, SGX_EINITTOKEN_SIZE) || > > + !access_ok(secs, PAGE_SIZE)); > > Ditto. The same as above, *trapnr should be set to X86_TRAP_GP before return -EINVAL, although for sigstruct, token, they are just normal memory, but not EPC. Sean, do you have comments?
On Tue, Apr 06, 2021 at 09:44:21AM +1200, Kai Huang wrote: > The intention was to catch KVM bug, since KVM is the only caller, and in current > implementation KVM won't call this function if @secs is not a valid userspace > pointer. But yes we can also return here, but in this case an exception number > must also be specified to *trapnr so that KVM can inject to guest. It's not that > straightforward to decide which exception should we inject, but I think #GP > should be OK. Please see below. Why should you inject anything in that case? AFAICT, you can handle the return value in __handle_encls_ecreate() and inject only when the return value is EFAULT. If it is another negative error value, you pass it back up to its caller, handle_encls_ecreate() which returns other error values like -ENOMEM too. Which means, its callchain can stomach negative values just fine.
On Tue, 6 Apr 2021 09:40:38 +0200 Borislav Petkov wrote: > On Tue, Apr 06, 2021 at 09:44:21AM +1200, Kai Huang wrote: > > The intention was to catch KVM bug, since KVM is the only caller, and in current > > implementation KVM won't call this function if @secs is not a valid userspace > > pointer. But yes we can also return here, but in this case an exception number > > must also be specified to *trapnr so that KVM can inject to guest. It's not that > > straightforward to decide which exception should we inject, but I think #GP > > should be OK. Please see below. > > Why should you inject anything in that case? > > AFAICT, you can handle the return value in __handle_encls_ecreate() and > inject only when the return value is EFAULT. If it is another negative > error value, you pass it back up to its caller, handle_encls_ecreate() > which returns other error values like -ENOMEM too. Which means, its > callchain can stomach negative values just fine. > OK. My thinking was that, returning negative error value basically means guest will be killed. For the case access_ok() fails for @secs or other user pointers, it seems killing guest is a little it overkill, but since this code's purpose is to catch KVM bug, I think killing guest is also OK from this perspective (like -ENOMEM case, it is kernel/kvm internal error). So yes I guess we can make handle_encls_xx() to stomach negative values, and only inject upon -EFAULT.
On Tue, Apr 06, 2021 at 08:59:58PM +1200, Kai Huang wrote: > OK. My thinking was that, returning negative error value basically means guest > will be killed. You need to define how you're going to handle invalid input from the guest. If that guest is considered malicious, then sure, killing it makes sense. > For the case access_ok() fails for @secs or other user pointers, it > seems killing guest is a little it overkill, So don't kill it then - just don't allow it to create an enclave because it is doing stupid crap.
On Tue, 6 Apr 2021 11:09:01 +0200 Borislav Petkov wrote: > On Tue, Apr 06, 2021 at 08:59:58PM +1200, Kai Huang wrote: > > OK. My thinking was that, returning negative error value basically means guest > > will be killed. > > You need to define how you're going to handle invalid input from the > guest. If that guest is considered malicious, then sure, killing it > makes sense. Such invalid input has already been handled in handle_encls_xx() before calling the two helpers in this patch. KVM returns to Qemu and let it decide whether to kill or not. The access_ok()s here are trying to catch KVM bug. > > > For the case access_ok() fails for @secs or other user pointers, it > > seems killing guest is a little it overkill, > > So don't kill it then - just don't allow it to create an enclave because > it is doing stupid crap. If so we'd better inject an exception to guest (and return 1) in KVM so guest can continue to run. Otherwise basically KVM will return to Qemu and let it decide (and basically it will kill guest). I think killing guest is also OK. KVM part patches needs to be updated, though, anyway.
On Tue, Apr 06, 2021 at 09:24:24PM +1200, Kai Huang wrote: > Such invalid input has already been handled in handle_encls_xx() before calling > the two helpers in this patch. KVM returns to Qemu and let it decide whether to > kill or not. The access_ok()s here are trying to catch KVM bug. Whatever they try to do, you cannot continue creating an enclave using invalid input, no matter whether you've warned or not. People do not stare at dmesg all the time. > If so we'd better inject an exception to guest (and return 1) in KVM so guest > can continue to run. Otherwise basically KVM will return to Qemu and let it > decide (and basically it will kill guest). > > I think killing guest is also OK. KVM part patches needs to be updated, though, > anyway. Ok, I'll make the changes and you can redo the KVM rest ontop. Thx.
On Tue, 6 Apr 2021 11:32:11 +0200 Borislav Petkov wrote: > On Tue, Apr 06, 2021 at 09:24:24PM +1200, Kai Huang wrote: > > Such invalid input has already been handled in handle_encls_xx() before calling > > the two helpers in this patch. KVM returns to Qemu and let it decide whether to > > kill or not. The access_ok()s here are trying to catch KVM bug. > > Whatever they try to do, you cannot continue creating an enclave using > invalid input, no matter whether you've warned or not. People do not > stare at dmesg all the time. > > > If so we'd better inject an exception to guest (and return 1) in KVM so guest > > can continue to run. Otherwise basically KVM will return to Qemu and let it > > decide (and basically it will kill guest). > > > > I think killing guest is also OK. KVM part patches needs to be updated, though, > > anyway. > > Ok, I'll make the changes and you can redo the KVM rest ontop. > Thank you!
On Tue, Apr 06, 2021 at 09:41:52PM +1200, Kai Huang wrote: > > Ok, I'll make the changes and you can redo the KVM rest ontop. > > > > Thank you! I.e., something like this: --- From: Sean Christopherson <sean.j.christopherson@intel.com> Date: Fri, 19 Mar 2021 20:23:08 +1300 Subject: [PATCH] x86/sgx: Add helpers to expose ECREATE and EINIT to KVM The host kernel must intercept ECREATE to impose policies on guests, and intercept EINIT to be able to write guest's virtual SGX_LEPUBKEYHASH MSR values to hardware before running guest's EINIT so it can run correctly according to hardware behavior. Provide wrappers around __ecreate() and __einit() to hide the ugliness of overloading the ENCLS return value to encode multiple error formats in a single int. KVM will trap-and-execute ECREATE and EINIT as part of SGX virtualization, and reflect ENCLS execution result to guest by setting up guest's GPRs, or on an exception, injecting the correct fault based on return value of __ecreate() and __einit(). Use host userspace addresses (provided by KVM based on guest physical address of ENCLS parameters) to execute ENCLS/EINIT when possible. Accesses to both EPC and memory originating from ENCLS are subject to segmentation and paging mechanisms. It's also possible to generate kernel mappings for ENCLS parameters by resolving PFN but using __uaccess_xx() is simpler. [ bp: Return early if the __user memory accesses fail. ] Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Kai Huang <kai.huang@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by: Jarkko Sakkinen <jarkko@kernel.org> Link: https://lkml.kernel.org/r/20e09daf559aa5e9e680a0b4b5fba940f1bad86e.1616136308.git.kai.huang@intel.com --- arch/x86/include/asm/sgx.h | 7 ++ arch/x86/kernel/cpu/sgx/virt.c | 117 +++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h index 3b025afec0a7..954042e04102 100644 --- a/arch/x86/include/asm/sgx.h +++ b/arch/x86/include/asm/sgx.h @@ -365,4 +365,11 @@ struct sgx_sigstruct { * comment! */ +#ifdef CONFIG_X86_SGX_KVM +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, + int *trapnr); +int sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs, u64 *lepubkeyhash, int *trapnr); +#endif + #endif /* _ASM_X86_SGX_H */ diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c index 259cc46ad78c..7d221eac716a 100644 --- a/arch/x86/kernel/cpu/sgx/virt.c +++ b/arch/x86/kernel/cpu/sgx/virt.c @@ -257,3 +257,120 @@ int __init sgx_vepc_init(void) return misc_register(&sgx_vepc_dev); } + +/** + * sgx_virt_ecreate() - Run ECREATE on behalf of guest + * @pageinfo: Pointer to PAGEINFO structure + * @secs: Userspace pointer to SECS page + * @trapnr: trap number injected to guest in case of ECREATE error + * + * Run ECREATE on behalf of guest after KVM traps ECREATE for the purpose + * of enforcing policies of guest's enclaves, and return the trap number + * which should be injected to guest in case of any ECREATE error. + * + * Return: + * - 0: ECREATE was successful. + * - <0: on error. + */ +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, + int *trapnr) +{ + int ret; + + /* + * @secs is an untrusted, userspace-provided address. It comes from + * KVM and is assumed to be a valid pointer which points somewhere in + * userspace. This can fault and call SGX or other fault handlers when + * userspace mapping @secs doesn't exist. + * + * Add a WARN() to make sure @secs is already valid userspace pointer + * from caller (KVM), who should already have handled invalid pointer + * case (for instance, made by malicious guest). All other checks, + * such as alignment of @secs, are deferred to ENCLS itself. + */ + if (WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE))) + return -EINVAL; + + __uaccess_begin(); + ret = __ecreate(pageinfo, (void *)secs); + __uaccess_end(); + + if (encls_faulted(ret)) { + *trapnr = ENCLS_TRAPNR(ret); + return -EFAULT; + } + + /* ECREATE doesn't return an error code, it faults or succeeds. */ + WARN_ON_ONCE(ret); + return 0; +} +EXPORT_SYMBOL_GPL(sgx_virt_ecreate); + +static int __sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs) +{ + int ret; + + /* + * Make sure all userspace pointers from caller (KVM) are valid. + * All other checks deferred to ENCLS itself. Also see comment + * for @secs in sgx_virt_ecreate(). + */ +#define SGX_EINITTOKEN_SIZE 304 + if (WARN_ON_ONCE(!access_ok(sigstruct, sizeof(struct sgx_sigstruct)) || + !access_ok(token, SGX_EINITTOKEN_SIZE) || + !access_ok(secs, PAGE_SIZE))) + return -EINVAL; + + __uaccess_begin(); + ret = __einit((void *)sigstruct, (void *)token, (void *)secs); + __uaccess_end(); + + return ret; +} + +/** + * sgx_virt_einit() - Run EINIT on behalf of guest + * @sigstruct: Userspace pointer to SIGSTRUCT structure + * @token: Userspace pointer to EINITTOKEN structure + * @secs: Userspace pointer to SECS page + * @lepubkeyhash: Pointer to guest's *virtual* SGX_LEPUBKEYHASH MSR values + * @trapnr: trap number injected to guest in case of EINIT error + * + * Run EINIT on behalf of guest after KVM traps EINIT. If SGX_LC is available + * in host, SGX driver may rewrite the hardware values at wish, therefore KVM + * needs to update hardware values to guest's virtual MSR values in order to + * ensure EINIT is executed with expected hardware values. + * + * Return: + * - 0: EINIT was successful. + * - <0: on error. + */ +int sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs, u64 *lepubkeyhash, int *trapnr) +{ + int ret; + + if (!cpu_feature_enabled(X86_FEATURE_SGX_LC)) { + ret = __sgx_virt_einit(sigstruct, token, secs); + } else { + preempt_disable(); + + sgx_update_lepubkeyhash(lepubkeyhash); + + ret = __sgx_virt_einit(sigstruct, token, secs); + preempt_enable(); + } + + /* Propagate up the error from the WARN_ON_ONCE in __sgx_virt_einit() */ + if (ret == -EINVAL) + return ret; + + if (encls_faulted(ret)) { + *trapnr = ENCLS_TRAPNR(ret); + return -EFAULT; + } + + return ret; +} +EXPORT_SYMBOL_GPL(sgx_virt_einit);
On Tue, 6 Apr 2021 19:08:58 +0200 Borislav Petkov wrote: > On Tue, Apr 06, 2021 at 09:41:52PM +1200, Kai Huang wrote: > > > Ok, I'll make the changes and you can redo the KVM rest ontop. > > > > > > > Thank you! > > I.e., something like this: Looks good. I'll update KVM part patches based on this one. Thanks. > > --- > From: Sean Christopherson <sean.j.christopherson@intel.com> > Date: Fri, 19 Mar 2021 20:23:08 +1300 > Subject: [PATCH] x86/sgx: Add helpers to expose ECREATE and EINIT to KVM > > The host kernel must intercept ECREATE to impose policies on guests, and > intercept EINIT to be able to write guest's virtual SGX_LEPUBKEYHASH MSR > values to hardware before running guest's EINIT so it can run correctly > according to hardware behavior. > > Provide wrappers around __ecreate() and __einit() to hide the ugliness > of overloading the ENCLS return value to encode multiple error formats > in a single int. KVM will trap-and-execute ECREATE and EINIT as part > of SGX virtualization, and reflect ENCLS execution result to guest by > setting up guest's GPRs, or on an exception, injecting the correct fault > based on return value of __ecreate() and __einit(). > > Use host userspace addresses (provided by KVM based on guest physical > address of ENCLS parameters) to execute ENCLS/EINIT when possible. > Accesses to both EPC and memory originating from ENCLS are subject to > segmentation and paging mechanisms. It's also possible to generate > kernel mappings for ENCLS parameters by resolving PFN but using > __uaccess_xx() is simpler. > > [ bp: Return early if the __user memory accesses fail. ] > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kai Huang <kai.huang@intel.com> > Signed-off-by: Borislav Petkov <bp@suse.de> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org> > Link: https://lkml.kernel.org/r/20e09daf559aa5e9e680a0b4b5fba940f1bad86e.1616136308.git.kai.huang@intel.com > --- > arch/x86/include/asm/sgx.h | 7 ++ > arch/x86/kernel/cpu/sgx/virt.c | 117 +++++++++++++++++++++++++++++++++ > 2 files changed, 124 insertions(+) > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h > index 3b025afec0a7..954042e04102 100644 > --- a/arch/x86/include/asm/sgx.h > +++ b/arch/x86/include/asm/sgx.h > @@ -365,4 +365,11 @@ struct sgx_sigstruct { > * comment! > */ > > +#ifdef CONFIG_X86_SGX_KVM > +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, > + int *trapnr); > +int sgx_virt_einit(void __user *sigstruct, void __user *token, > + void __user *secs, u64 *lepubkeyhash, int *trapnr); > +#endif > + > #endif /* _ASM_X86_SGX_H */ > diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c > index 259cc46ad78c..7d221eac716a 100644 > --- a/arch/x86/kernel/cpu/sgx/virt.c > +++ b/arch/x86/kernel/cpu/sgx/virt.c > @@ -257,3 +257,120 @@ int __init sgx_vepc_init(void) > > return misc_register(&sgx_vepc_dev); > } > + > +/** > + * sgx_virt_ecreate() - Run ECREATE on behalf of guest > + * @pageinfo: Pointer to PAGEINFO structure > + * @secs: Userspace pointer to SECS page > + * @trapnr: trap number injected to guest in case of ECREATE error > + * > + * Run ECREATE on behalf of guest after KVM traps ECREATE for the purpose > + * of enforcing policies of guest's enclaves, and return the trap number > + * which should be injected to guest in case of any ECREATE error. > + * > + * Return: > + * - 0: ECREATE was successful. > + * - <0: on error. > + */ > +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, > + int *trapnr) > +{ > + int ret; > + > + /* > + * @secs is an untrusted, userspace-provided address. It comes from > + * KVM and is assumed to be a valid pointer which points somewhere in > + * userspace. This can fault and call SGX or other fault handlers when > + * userspace mapping @secs doesn't exist. > + * > + * Add a WARN() to make sure @secs is already valid userspace pointer > + * from caller (KVM), who should already have handled invalid pointer > + * case (for instance, made by malicious guest). All other checks, > + * such as alignment of @secs, are deferred to ENCLS itself. > + */ > + if (WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE))) > + return -EINVAL; > + > + __uaccess_begin(); > + ret = __ecreate(pageinfo, (void *)secs); > + __uaccess_end(); > + > + if (encls_faulted(ret)) { > + *trapnr = ENCLS_TRAPNR(ret); > + return -EFAULT; > + } > + > + /* ECREATE doesn't return an error code, it faults or succeeds. */ > + WARN_ON_ONCE(ret); > + return 0; > +} > +EXPORT_SYMBOL_GPL(sgx_virt_ecreate); > + > +static int __sgx_virt_einit(void __user *sigstruct, void __user *token, > + void __user *secs) > +{ > + int ret; > + > + /* > + * Make sure all userspace pointers from caller (KVM) are valid. > + * All other checks deferred to ENCLS itself. Also see comment > + * for @secs in sgx_virt_ecreate(). > + */ > +#define SGX_EINITTOKEN_SIZE 304 > + if (WARN_ON_ONCE(!access_ok(sigstruct, sizeof(struct sgx_sigstruct)) || > + !access_ok(token, SGX_EINITTOKEN_SIZE) || > + !access_ok(secs, PAGE_SIZE))) > + return -EINVAL; > + > + __uaccess_begin(); > + ret = __einit((void *)sigstruct, (void *)token, (void *)secs); > + __uaccess_end(); > + > + return ret; > +} > + > +/** > + * sgx_virt_einit() - Run EINIT on behalf of guest > + * @sigstruct: Userspace pointer to SIGSTRUCT structure > + * @token: Userspace pointer to EINITTOKEN structure > + * @secs: Userspace pointer to SECS page > + * @lepubkeyhash: Pointer to guest's *virtual* SGX_LEPUBKEYHASH MSR values > + * @trapnr: trap number injected to guest in case of EINIT error > + * > + * Run EINIT on behalf of guest after KVM traps EINIT. If SGX_LC is available > + * in host, SGX driver may rewrite the hardware values at wish, therefore KVM > + * needs to update hardware values to guest's virtual MSR values in order to > + * ensure EINIT is executed with expected hardware values. > + * > + * Return: > + * - 0: EINIT was successful. > + * - <0: on error. > + */ > +int sgx_virt_einit(void __user *sigstruct, void __user *token, > + void __user *secs, u64 *lepubkeyhash, int *trapnr) > +{ > + int ret; > + > + if (!cpu_feature_enabled(X86_FEATURE_SGX_LC)) { > + ret = __sgx_virt_einit(sigstruct, token, secs); > + } else { > + preempt_disable(); > + > + sgx_update_lepubkeyhash(lepubkeyhash); > + > + ret = __sgx_virt_einit(sigstruct, token, secs); > + preempt_enable(); > + } > + > + /* Propagate up the error from the WARN_ON_ONCE in __sgx_virt_einit() */ > + if (ret == -EINVAL) > + return ret; > + > + if (encls_faulted(ret)) { > + *trapnr = ENCLS_TRAPNR(ret); > + return -EFAULT; > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(sgx_virt_einit); > -- > 2.29.2 > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h index 3b025afec0a7..954042e04102 100644 --- a/arch/x86/include/asm/sgx.h +++ b/arch/x86/include/asm/sgx.h @@ -365,4 +365,11 @@ struct sgx_sigstruct { * comment! */ +#ifdef CONFIG_X86_SGX_KVM +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, + int *trapnr); +int sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs, u64 *lepubkeyhash, int *trapnr); +#endif + #endif /* _ASM_X86_SGX_H */ diff --git a/arch/x86/kernel/cpu/sgx/virt.c b/arch/x86/kernel/cpu/sgx/virt.c index 29d8d28b4695..eaff7d72e47f 100644 --- a/arch/x86/kernel/cpu/sgx/virt.c +++ b/arch/x86/kernel/cpu/sgx/virt.c @@ -258,3 +258,112 @@ int __init sgx_vepc_init(void) return misc_register(&sgx_vepc_dev); } + +/** + * sgx_virt_ecreate() - Run ECREATE on behalf of guest + * @pageinfo: Pointer to PAGEINFO structure + * @secs: Userspace pointer to SECS page + * @trapnr: trap number injected to guest in case of ECREATE error + * + * Run ECREATE on behalf of guest after KVM traps ECREATE for the purpose + * of enforcing policies of guest's enclaves, and return the trap number + * which should be injected to guest in case of any ECREATE error. + * + * Return: + * - 0: ECREATE was successful. + * - -EFAULT: ECREATE returned error. + */ +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs, + int *trapnr) +{ + int ret; + + /* + * @secs is an untrusted, userspace-provided address. It comes from + * KVM and is assumed to be a valid pointer which points somewhere in + * userspace. This can fault and call SGX or other fault handlers when + * userspace mapping @secs doesn't exist. + * + * Add a WARN() to make sure @secs is already valid userspace pointer + * from caller (KVM), who should already have handled invalid pointer + * case (for instance, made by malicious guest). All other checks, + * such as alignment of @secs, are deferred to ENCLS itself. + */ + WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)); + __uaccess_begin(); + ret = __ecreate(pageinfo, (void *)secs); + __uaccess_end(); + + if (encls_faulted(ret)) { + *trapnr = ENCLS_TRAPNR(ret); + return -EFAULT; + } + + /* ECREATE doesn't return an error code, it faults or succeeds. */ + WARN_ON_ONCE(ret); + return 0; +} +EXPORT_SYMBOL_GPL(sgx_virt_ecreate); + +static int __sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs) +{ + int ret; + + /* + * Make sure all userspace pointers from caller (KVM) are valid. + * All other checks deferred to ENCLS itself. Also see comment + * for @secs in sgx_virt_ecreate(). + */ +#define SGX_EINITTOKEN_SIZE 304 + WARN_ON_ONCE(!access_ok(sigstruct, sizeof(struct sgx_sigstruct)) || + !access_ok(token, SGX_EINITTOKEN_SIZE) || + !access_ok(secs, PAGE_SIZE)); + __uaccess_begin(); + ret = __einit((void *)sigstruct, (void *)token, (void *)secs); + __uaccess_end(); + + return ret; +} + +/** + * sgx_virt_einit() - Run EINIT on behalf of guest + * @sigstruct: Userspace pointer to SIGSTRUCT structure + * @token: Userspace pointer to EINITTOKEN structure + * @secs: Userspace pointer to SECS page + * @lepubkeyhash: Pointer to guest's *virtual* SGX_LEPUBKEYHASH MSR values + * @trapnr: trap number injected to guest in case of EINIT error + * + * Run EINIT on behalf of guest after KVM traps EINIT. If SGX_LC is available + * in host, SGX driver may rewrite the hardware values at wish, therefore KVM + * needs to update hardware values to guest's virtual MSR values in order to + * ensure EINIT is executed with expected hardware values. + * + * Return: + * - 0: EINIT was successful. + * - -EFAULT: EINIT returned error. + */ +int sgx_virt_einit(void __user *sigstruct, void __user *token, + void __user *secs, u64 *lepubkeyhash, int *trapnr) +{ + int ret; + + if (!boot_cpu_has(X86_FEATURE_SGX_LC)) { + ret = __sgx_virt_einit(sigstruct, token, secs); + } else { + preempt_disable(); + + sgx_update_lepubkeyhash(lepubkeyhash); + + ret = __sgx_virt_einit(sigstruct, token, secs); + preempt_enable(); + } + + if (encls_faulted(ret)) { + *trapnr = ENCLS_TRAPNR(ret); + return -EFAULT; + } + + return ret; +} +EXPORT_SYMBOL_GPL(sgx_virt_einit);