On Wed, Feb 03, 2021, Edgecombe, Rick P wrote: > On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote: > > +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?