On Wed, Feb 03, 2021, Edgecombe, Rick P wrote:

On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote:

quoted

+static int handle_encls_ecreate(struct kvm_vcpu *vcpu)
+{
+       unsigned long a_hva, m_hva, x_hva, s_hva, secs_hva;
+       struct kvm_cpuid_entry2 *sgx_12_0, *sgx_12_1;
+       gpa_t metadata_gpa, contents_gpa, secs_gpa;
+       struct sgx_pageinfo pageinfo;
+       gva_t pageinfo_gva, secs_gva;
+       u64 attributes, xfrm, size;
+       struct x86_exception ex;
+       u8 max_size_log2;
+       u32 miscselect;
+       int trapnr, r;
+
+       sgx_12_0 = kvm_find_cpuid_entry(vcpu, 0x12, 0);
+       sgx_12_1 = kvm_find_cpuid_entry(vcpu, 0x12, 1);
+       if (!sgx_12_0 || !sgx_12_1) {
+               kvm_inject_gp(vcpu, 0);
+               return 1;
+       }
+
+       if (sgx_get_encls_gva(vcpu, kvm_rbx_read(vcpu), 32, 32,
&pageinfo_gva) ||
+           sgx_get_encls_gva(vcpu, kvm_rcx_read(vcpu), 4096, 4096,
&secs_gva))
+               return 1;
+
+       /*
+        * Copy the PAGEINFO to local memory, its pointers need to be
+        * translated, i.e. we need to do a deep copy/translate.
+        */
+       r = kvm_read_guest_virt(vcpu, pageinfo_gva, &pageinfo,
+                               sizeof(pageinfo), &ex);
+       if (r == X86EMUL_PROPAGATE_FAULT) {
+               kvm_inject_emulated_page_fault(vcpu, &ex);
+               return 1;
+       } else if (r != X86EMUL_CONTINUE) {
+               sgx_handle_emulation_failure(vcpu, pageinfo_gva,
size);
+               return 0;
+       }
+
+       /*
+        * Verify alignment early.  This conveniently avoids having
to worry
+        * about page splits on userspace addresses.
+        */
+       if (!IS_ALIGNED(pageinfo.metadata, 64) ||
+           !IS_ALIGNED(pageinfo.contents, 4096)) {
+               kvm_inject_gp(vcpu, 0);
+               return 1;
+       }
+
+       /*
+        * Translate the SECINFO, SOURCE and SECS pointers from GVA
to GPA.
+        * Resume the guest on failure to inject a #PF.
+        */
+       if (sgx_gva_to_gpa(vcpu, pageinfo.metadata, false,
&metadata_gpa) ||
+           sgx_gva_to_gpa(vcpu, pageinfo.contents, false,
&contents_gpa) ||
+           sgx_gva_to_gpa(vcpu, secs_gva, true, &secs_gpa))
+               return 1;
+

Do pageinfo.metadata and pageinfo.contents need cannonical checks here?

Bugger, yes.  So much boilerplate needed in this code :-/

Maybe add yet another helper to do alignment+canonical checks, up where the
IS_ALIGNED() calls are?