Re: [RFC PATCH v3 23/27] KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions

Edgecombe, Rick P
2021-02-03T18:47:56Z

On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote:
> +       /* Exit to userspace if copying from a host userspace address
> fails. */
> +       if (sgx_read_hva(vcpu, m_hva, &miscselect,
> sizeof(miscselect)) ||
> +           sgx_read_hva(vcpu, a_hva, &attributes,
> sizeof(attributes)) ||
> +           sgx_read_hva(vcpu, x_hva, &xfrm, sizeof(xfrm)) ||
> +           sgx_read_hva(vcpu, s_hva, &size, sizeof(size)))
> +               return 0;
> +
> +       /* Enforce restriction of access to the PROVISIONKEY. */
> +       if (!vcpu->kvm->arch.sgx_provisioning_allowed &&
> +           (attributes & SGX_ATTR_PROVISIONKEY)) {
> +               if (sgx_12_1->eax & SGX_ATTR_PROVISIONKEY)
> +                       pr_warn_once("KVM: SGX PROVISIONKEY
> advertised but not allowed\n");
> +               kvm_inject_gp(vcpu, 0);
> +               return 1;
> +       }
> +
> +       /* Enforce CPUID restrictions on MISCSELECT, ATTRIBUTES and
> XFRM. */
> +       if ((u32)miscselect & ~sgx_12_0->ebx ||
> +           (u32)attributes & ~sgx_12_1->eax ||
> +           (u32)(attributes >> 32) & ~sgx_12_1->ebx ||
> +           (u32)xfrm & ~sgx_12_1->ecx ||
> +           (u32)(xfrm >> 32) & ~sgx_12_1->edx) {
> +               kvm_inject_gp(vcpu, 0);
> +               return 1;
> +       }

Don't you need to deep copy the pageinfo.contents struct as well?
Otherwise the guest could change these after they were checked.

But it seems it is checked by the HW and something is caught that would
inject a GP anyway? Can you elaborate on the importance of these
checks?