lkml.org 
[lkml]   [2023]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH RFC v8 54/56] x86/sev: Add KVM commands for instance certs
    From
    Hi Mike,

    On 20/02/2023 20:38, Michael Roth wrote:
    > From: Dionna Glaze <dionnaglaze@google.com>
    >
    > The /dev/sev device has the ability to store host-wide certificates for
    > the key used by the AMD-SP for SEV-SNP attestation report signing,
    > but for hosts that want to specify additional certificates that are
    > specific to the image launched in a VM, a different way is needed to
    > communicate those certificates.
    >
    > Add two new KVM ioctl to handle this: KVM_SEV_SNP_{GET,SET}_CERTS
    >
    > The certificates that are set with this command are expected to follow
    > the same format as the host certificates, but that format is opaque
    > to the kernel.
    >
    > The new behavior for custom certificates is that the extended guest
    > request command will now return the overridden certificates if they
    > were installed for the instance. The error condition for a too small
    > data buffer is changed to return the overridden certificate data size
    > if there is an overridden certificate set installed.
    >
    > Setting a 0 length certificate returns the system state to only return
    > the host certificates on an extended guest request.
    >
    > Also increase the SEV_FW_BLOB_MAX_SIZE another 4K page to allow space
    > for an extra certificate.
    >
    > Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
    > Cc: Paolo Bonzini <pbonzini@redhat.com>
    >
    > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
    > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
    > [mdr: remove used of "we" and "this patch" in commit log]
    > Signed-off-by: Michael Roth <michael.roth@amd.com>
    > ---
    > arch/x86/kvm/svm/sev.c | 111 ++++++++++++++++++++++++++++++++++++++-
    > arch/x86/kvm/svm/svm.h | 1 +
    > include/linux/psp-sev.h | 2 +-
    > include/uapi/linux/kvm.h | 12 +++++
    > 4 files changed, 123 insertions(+), 3 deletions(-)
    >
    > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
    > index 70d5650d8d95..18b64b7005e7 100644
    > --- a/arch/x86/kvm/svm/sev.c
    > +++ b/arch/x86/kvm/svm/sev.c
    > @@ -2089,6 +2089,7 @@ static void *snp_context_create(struct kvm *kvm, struct kvm_sev_cmd *argp)
    > goto e_free;
    >
    > sev->snp_certs_data = certs_data;
    > + sev->snp_certs_len = 0;
    >
    > return context;
    >
    > @@ -2404,6 +2405,86 @@ static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
    > return ret;
    > }
    >
    > +static int snp_get_instance_certs(struct kvm *kvm, struct kvm_sev_cmd *argp)
    > +{
    > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
    > + struct kvm_sev_snp_get_certs params;
    > +
    > + if (!sev_snp_guest(kvm))
    > + return -ENOTTY;
    > +
    > + if (!sev->snp_context)
    > + return -EINVAL;
    > +
    > + if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
    > + sizeof(params)))
    > + return -EFAULT;
    > +
    > + /* No instance certs set. */
    > + if (!sev->snp_certs_len)
    > + return -ENOENT;
    > +
    > + if (params.certs_len < sev->snp_certs_len) {
    > + /* Output buffer too small. Return the required size. */
    > + params.certs_len = sev->snp_certs_len;
    > +
    > + if (copy_to_user((void __user *)(uintptr_t)argp->data, &params,
    > + sizeof(params)))
    > + return -EFAULT;
    > +
    > + return -EINVAL;
    > + }
    > +
    > + if (copy_to_user((void __user *)(uintptr_t)params.certs_uaddr,
    > + sev->snp_certs_data, sev->snp_certs_len))
    > + return -EFAULT;
    > +
    > + return 0;
    > +}
    > +
    > +static int snp_set_instance_certs(struct kvm *kvm, struct kvm_sev_cmd *argp)
    > +{
    > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
    > + unsigned long length = SEV_FW_BLOB_MAX_SIZE;
    > + void *to_certs = sev->snp_certs_data;
    > + struct kvm_sev_snp_set_certs params;
    > +
    > + if (!sev_snp_guest(kvm))
    > + return -ENOTTY;
    > +
    > + if (!sev->snp_context)
    > + return -EINVAL;
    > +
    > + if (copy_from_user(&params, (void __user *)(uintptr_t)argp->data,
    > + sizeof(params)))
    > + return -EFAULT;
    > +
    > + if (params.certs_len > SEV_FW_BLOB_MAX_SIZE)
    > + return -EINVAL;
    > +
    > + /*
    > + * Setting a length of 0 is the same as "uninstalling" instance-
    > + * specific certificates.
    > + */
    > + if (params.certs_len == 0) {
    > + sev->snp_certs_len = 0;
    > + return 0;
    > + }
    > +
    > + /* Page-align the length */
    > + length = (params.certs_len + PAGE_SIZE - 1) & PAGE_MASK;
    > +

    In comments on v7 [1] Dionna agreed adding cleanup here:

    /* The size could shrink and leave garbage at the end. */
    memset(sev->snp_certs_data, 0, SEV_FW_BLOB_MAX_SIZE);

    (we can use 'to_certs' in the first argument)

    but it wasn't added in v8.

    -Dov

    [1] https://lore.kernel.org/linux-coco/CAAH4kHYOtzgqSTZQFcRiZwPLCkLAThjsCMdjUCdsBTiP=W0Vxw@mail.gmail.com/


    > + if (copy_from_user(to_certs,
    > + (void __user *)(uintptr_t)params.certs_uaddr,
    > + params.certs_len)) {
    > + return -EFAULT;
    > + }
    > +
    > + sev->snp_certs_len = length;
    > +
    > + return 0;
    > +}
    > +
    > int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
    > {
    > struct kvm_sev_cmd sev_cmd;
    > @@ -2503,6 +2584,12 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
    > case KVM_SEV_SNP_LAUNCH_FINISH:
    > r = snp_launch_finish(kvm, &sev_cmd);
    > break;
    > + case KVM_SEV_SNP_GET_CERTS:
    > + r = snp_get_instance_certs(kvm, &sev_cmd);
    > + break;
    > + case KVM_SEV_SNP_SET_CERTS:
    > + r = snp_set_instance_certs(kvm, &sev_cmd);
    > + break;
    > default:
    > r = -EINVAL;
    > goto out;
    > @@ -3550,8 +3637,28 @@ static void snp_handle_ext_guest_request(struct vcpu_svm *svm, gpa_t req_gpa, gp
    > if (rc)
    > goto unlock;
    >
    > - rc = snp_guest_ext_guest_request(&req, (unsigned long)sev->snp_certs_data,
    > - &data_npages, &err);
    > + /*
    > + * If the VMM has overridden the certs, then change the error message
    > + * if the size is inappropriate for the override. Otherwise, use a
    > + * regular guest request and copy back the instance certs.
    > + */
    > + if (sev->snp_certs_len) {
    > + if ((data_npages << PAGE_SHIFT) < sev->snp_certs_len) {
    > + rc = -EINVAL;
    > + err = SNP_GUEST_REQ_INVALID_LEN;
    > + goto datalen;
    > + }
    > + rc = sev_issue_cmd(kvm, SEV_CMD_SNP_GUEST_REQUEST, &req,
    > + (int *)&err);
    > + } else {
    > + rc = snp_guest_ext_guest_request(&req,
    > + (unsigned long)sev->snp_certs_data,
    > + &data_npages, &err);
    > + }
    > +datalen:
    > + if (sev->snp_certs_len)
    > + data_npages = sev->snp_certs_len >> PAGE_SHIFT;
    > +
    > if (rc) {
    > /*
    > * If buffer length is small then return the expected
    > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
    > index 221b38d3c845..dced46559508 100644
    > --- a/arch/x86/kvm/svm/svm.h
    > +++ b/arch/x86/kvm/svm/svm.h
    > @@ -94,6 +94,7 @@ struct kvm_sev_info {
    > u64 snp_init_flags;
    > void *snp_context; /* SNP guest context page */
    > void *snp_certs_data;
    > + unsigned int snp_certs_len; /* Size of instance override for certs */
    > struct mutex guest_req_lock; /* Lock for guest request handling */
    >
    > u64 sev_features; /* Features set at VMSA creation */
    > diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
    > index 92116e2b74fd..3b28b78938f6 100644
    > --- a/include/linux/psp-sev.h
    > +++ b/include/linux/psp-sev.h
    > @@ -22,7 +22,7 @@
    > #define __psp_pa(x) __pa(x)
    > #endif
    >
    > -#define SEV_FW_BLOB_MAX_SIZE 0x4000 /* 16KB */
    > +#define SEV_FW_BLOB_MAX_SIZE 0x5000 /* 20KB */
    >
    > /**
    > * SEV platform state
    > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
    > index 6e684bf5f723..ad7e24e43547 100644
    > --- a/include/uapi/linux/kvm.h
    > +++ b/include/uapi/linux/kvm.h
    > @@ -1928,6 +1928,8 @@ enum sev_cmd_id {
    > KVM_SEV_SNP_LAUNCH_START,
    > KVM_SEV_SNP_LAUNCH_UPDATE,
    > KVM_SEV_SNP_LAUNCH_FINISH,
    > + KVM_SEV_SNP_GET_CERTS,
    > + KVM_SEV_SNP_SET_CERTS,
    >
    > KVM_SEV_NR_MAX,
    > };
    > @@ -2075,6 +2077,16 @@ struct kvm_sev_snp_launch_finish {
    > __u8 pad[6];
    > };
    >
    > +struct kvm_sev_snp_get_certs {
    > + __u64 certs_uaddr;
    > + __u64 certs_len;
    > +};
    > +
    > +struct kvm_sev_snp_set_certs {
    > + __u64 certs_uaddr;
    > + __u64 certs_len;
    > +};
    > +
    > #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
    > #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
    > #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)

    \
     
     \ /
      Last update: 2023-03-27 00:30    [W:3.975 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site