| Message ID | 20210406224952.4177376-1-seanjc@google.com |
|---|---|
| Headers | show |
| Series | ccp: KVM: SVM: Use stack for SEV command buffers | expand |
Le 07/04/2021 à 00:49, Sean Christopherson a écrit : > For commands with small input/output buffers, use the local stack to > "allocate" the structures used to communicate with the PSP. Now that > __sev_do_cmd_locked() gracefully handles vmalloc'd buffers, there's no > reason to avoid using the stack, e.g. CONFIG_VMAP_STACK=y will just work. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > drivers/crypto/ccp/sev-dev.c | 122 ++++++++++++++--------------------- > 1 file changed, 47 insertions(+), 75 deletions(-) > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index 4aedbdaffe90..bb0d6de071e6 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -396,7 +396,7 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable) > { > struct sev_device *sev = psp_master->sev_data; > struct sev_user_data_pek_csr input; > - struct sev_data_pek_csr *data; > + struct sev_data_pek_csr data; struct sev_data_pek_csr data = {0, 0}; > void __user *input_address; > void *blob = NULL; > int ret; > @@ -407,29 +407,24 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable) > if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > return -EFAULT; > > - data = kzalloc(sizeof(*data), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > - > /* userspace wants to query CSR length */ > - if (!input.address || !input.length) > + if (!input.address || !input.length) { > + data.address = 0; > + data.len = 0; With the change proposed above, those two new lines are unneeded. > goto cmd; > + } > > /* allocate a physically contiguous buffer to store the CSR blob */ > input_address = (void __user *)input.address; > - if (input.length > SEV_FW_BLOB_MAX_SIZE) { > - ret = -EFAULT; > - goto e_free; > - } > + if (input.length > SEV_FW_BLOB_MAX_SIZE) > + return -EFAULT; > > blob = kmalloc(input.length, GFP_KERNEL); > - if (!blob) { > - ret = -ENOMEM; > - goto e_free; > - } > + if (!blob) > + return -ENOMEM; > > - data->address = __psp_pa(blob); > - data->len = input.length; > + data.address = __psp_pa(blob); > + data.len = input.length; > > cmd: > if (sev->state == SEV_STATE_UNINIT) { > @@ -438,10 +433,10 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable) > goto e_free_blob; > } > > - ret = __sev_do_cmd_locked(SEV_CMD_PEK_CSR, data, &argp->error); > + ret = __sev_do_cmd_locked(SEV_CMD_PEK_CSR, &data, &argp->error); > > /* If we query the CSR length, FW responded with expected data. */ > - input.length = data->len; > + input.length = data.len; > > if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { > ret = -EFAULT; > @@ -455,8 +450,6 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable) > > e_free_blob: > kfree(blob); > -e_free: > - kfree(data); > return ret; > } > > @@ -588,7 +581,7 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable) > { > struct sev_device *sev = psp_master->sev_data; > struct sev_user_data_pek_cert_import input; > - struct sev_data_pek_cert_import *data; > + struct sev_data_pek_cert_import data; > void *pek_blob, *oca_blob; > int ret; > > @@ -598,19 +591,14 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable) > if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > return -EFAULT; > > - data = kzalloc(sizeof(*data), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > - > /* copy PEK certificate blobs from userspace */ > pek_blob = psp_copy_user_blob(input.pek_cert_address, input.pek_cert_len); > - if (IS_ERR(pek_blob)) { > - ret = PTR_ERR(pek_blob); > - goto e_free; > - } > + if (IS_ERR(pek_blob)) > + return PTR_ERR(pek_blob); > > - data->pek_cert_address = __psp_pa(pek_blob); > - data->pek_cert_len = input.pek_cert_len; > + data.reserved = 0; > + data.pek_cert_address = __psp_pa(pek_blob); > + data.pek_cert_len = input.pek_cert_len; > > /* copy PEK certificate blobs from userspace */ > oca_blob = psp_copy_user_blob(input.oca_cert_address, input.oca_cert_len); > @@ -619,8 +607,8 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable) > goto e_free_pek; > } > > - data->oca_cert_address = __psp_pa(oca_blob); > - data->oca_cert_len = input.oca_cert_len; > + data.oca_cert_address = __psp_pa(oca_blob); > + data.oca_cert_len = input.oca_cert_len; > > /* If platform is not in INIT state then transition it to INIT */ > if (sev->state != SEV_STATE_INIT) { > @@ -629,21 +617,19 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable) > goto e_free_oca; > } > > - ret = __sev_do_cmd_locked(SEV_CMD_PEK_CERT_IMPORT, data, &argp->error); > + ret = __sev_do_cmd_locked(SEV_CMD_PEK_CERT_IMPORT, &data, &argp->error); > > e_free_oca: > kfree(oca_blob); > e_free_pek: > kfree(pek_blob); > -e_free: > - kfree(data); > return ret; > } > > static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp) > { > struct sev_user_data_get_id2 input; > - struct sev_data_get_id *data; > + struct sev_data_get_id data; > void __user *input_address; > void *id_blob = NULL; > int ret; > @@ -657,28 +643,25 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp) > > input_address = (void __user *)input.address; > > - data = kzalloc(sizeof(*data), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > - > if (input.address && input.length) { > id_blob = kmalloc(input.length, GFP_KERNEL); > - if (!id_blob) { > - kfree(data); > + if (!id_blob) > return -ENOMEM; > - } > > - data->address = __psp_pa(id_blob); > - data->len = input.length; > + data.address = __psp_pa(id_blob); > + data.len = input.length; > + } else { > + data.address = 0; > + data.len = 0; > } > > - ret = __sev_do_cmd_locked(SEV_CMD_GET_ID, data, &argp->error); > + ret = __sev_do_cmd_locked(SEV_CMD_GET_ID, &data, &argp->error); > > /* > * Firmware will return the length of the ID value (either the minimum > * required length or the actual length written), return it to the user. > */ > - input.length = data->len; > + input.length = data.len; > > if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { > ret = -EFAULT; > @@ -686,7 +669,7 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp) > } > > if (id_blob) { > - if (copy_to_user(input_address, id_blob, data->len)) { > + if (copy_to_user(input_address, id_blob, data.len)) { > ret = -EFAULT; > goto e_free; > } > @@ -694,7 +677,6 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp) > > e_free: > kfree(id_blob); > - kfree(data); > > return ret; > } > @@ -744,7 +726,7 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable) > struct sev_device *sev = psp_master->sev_data; > struct sev_user_data_pdh_cert_export input; > void *pdh_blob = NULL, *cert_blob = NULL; > - struct sev_data_pdh_cert_export *data; > + struct sev_data_pdh_cert_export data; struct sev_data_pdh_cert_export data = {0, 0, 0, 0, 0}; > void __user *input_cert_chain_address; > void __user *input_pdh_cert_address; > int ret; > @@ -762,9 +744,7 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable) > if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) > return -EFAULT; > > - data = kzalloc(sizeof(*data), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > + memset(&data, 0, sizeof(data)); You can avoid that memset by initing at declaration, see above. > > /* Userspace wants to query the certificate length. */ > if (!input.pdh_cert_address || > @@ -776,25 +756,19 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable) > input_cert_chain_address = (void __user *)input.cert_chain_address; > > /* Allocate a physically contiguous buffer to store the PDH blob. */ > - if (input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) { > - ret = -EFAULT; > - goto e_free; > - } > + if (input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) > + return -EFAULT; > > /* Allocate a physically contiguous buffer to store the cert chain blob. */ > - if (input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) { > - ret = -EFAULT; > - goto e_free; > - } > + if (input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) > + return -EFAULT; > > pdh_blob = kmalloc(input.pdh_cert_len, GFP_KERNEL); > - if (!pdh_blob) { > - ret = -ENOMEM; > - goto e_free; > - } > + if (!pdh_blob) > + return -ENOMEM; > > - data->pdh_cert_address = __psp_pa(pdh_blob); > - data->pdh_cert_len = input.pdh_cert_len; > + data.pdh_cert_address = __psp_pa(pdh_blob); > + data.pdh_cert_len = input.pdh_cert_len; > > cert_blob = kmalloc(input.cert_chain_len, GFP_KERNEL); > if (!cert_blob) { > @@ -802,15 +776,15 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable) > goto e_free_pdh; > } > > - data->cert_chain_address = __psp_pa(cert_blob); > - data->cert_chain_len = input.cert_chain_len; > + data.cert_chain_address = __psp_pa(cert_blob); > + data.cert_chain_len = input.cert_chain_len; > > cmd: > - ret = __sev_do_cmd_locked(SEV_CMD_PDH_CERT_EXPORT, data, &argp->error); > + ret = __sev_do_cmd_locked(SEV_CMD_PDH_CERT_EXPORT, &data, &argp->error); > > /* If we query the length, FW responded with expected data. */ > - input.cert_chain_len = data->cert_chain_len; > - input.pdh_cert_len = data->pdh_cert_len; > + input.cert_chain_len = data.cert_chain_len; > + input.pdh_cert_len = data.pdh_cert_len; > > if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) { > ret = -EFAULT; > @@ -835,8 +809,6 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable) > kfree(cert_blob); > e_free_pdh: > kfree(pdh_blob); > -e_free: > - kfree(data); > return ret; > } > >
Le 07/04/2021 à 00:49, Sean Christopherson a écrit : > Drop the dedicated init_cmd_buf and instead use a local variable. Now > that the low level helper uses an internal buffer for all commands, > using the stack for the upper layers is safe even when running with > CONFIG_VMAP_STACK=y. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > drivers/crypto/ccp/sev-dev.c | 10 ++++++---- > drivers/crypto/ccp/sev-dev.h | 1 - > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index e54774b0d637..9ff28df03030 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -233,6 +233,7 @@ static int sev_do_cmd(int cmd, void *data, int *psp_ret) > static int __sev_platform_init_locked(int *error) > { > struct psp_device *psp = psp_master; > + struct sev_data_init data; struct sev_data_init data = {0, 0, 0, 0}; > struct sev_device *sev; > int rc = 0; > > @@ -244,6 +245,7 @@ static int __sev_platform_init_locked(int *error) > if (sev->state == SEV_STATE_INIT) > return 0; > > + memset(&data, 0, sizeof(data)); Not needed. > if (sev_es_tmr) { > u64 tmr_pa; > > @@ -253,12 +255,12 @@ static int __sev_platform_init_locked(int *error) > */ > tmr_pa = __pa(sev_es_tmr); > > - sev->init_cmd_buf.flags |= SEV_INIT_FLAGS_SEV_ES; > - sev->init_cmd_buf.tmr_address = tmr_pa; > - sev->init_cmd_buf.tmr_len = SEV_ES_TMR_SIZE; > + data.flags |= SEV_INIT_FLAGS_SEV_ES; > + data.tmr_address = tmr_pa; > + data.tmr_len = SEV_ES_TMR_SIZE; > } > > - rc = __sev_do_cmd_locked(SEV_CMD_INIT, &sev->init_cmd_buf, error); > + rc = __sev_do_cmd_locked(SEV_CMD_INIT, &data, error); > if (rc) > return rc; > > diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h > index 0fd21433f627..666c21eb81ab 100644 > --- a/drivers/crypto/ccp/sev-dev.h > +++ b/drivers/crypto/ccp/sev-dev.h > @@ -46,7 +46,6 @@ struct sev_device { > unsigned int int_rcvd; > wait_queue_head_t int_queue; > struct sev_misc_dev *misc; > - struct sev_data_init init_cmd_buf; > > u8 api_major; > u8 api_minor; >
On 4/6/21 5:49 PM, Sean Christopherson wrote: > This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd > command buffers by copying _all_ incoming data pointers to an internal > buffer before sending the command to the PSP. The SEV driver and KVM are > then converted to use the stack for all command buffers. > > Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere > near enough about the PSP to give it the right input. > > v2: > - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs > sharing SEV context"). > - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh] > - Allocate a full page for the buffer. [Brijesh] > - Drop one set of the "!"s. [Christophe] > - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary > patch (definitely feel free to drop the patch if it's not worth > backporting). [Christophe] > - s/intput/input/. [Tom] > - Add a patch to free "sev" if init fails. This is not strictly > necessary (I think; I suck horribly when it comes to the driver > framework). But it felt wrong to not free cmd_buf on failure, and > even more wrong to free cmd_buf but not sev. > > v1: > - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20210402233702.3291792-1-seanjc%40google.com&data=04%7C01%7Cbrijesh.singh%40amd.com%7C051db746fc2048e06acb08d8f94e527b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637533462083069551%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=bbNHBXMO1RWh8i4siTYkv4P92Ph5C7SnAZ3uTPsxgvg%3D&reserved=0 > > Sean Christopherson (8): > crypto: ccp: Free SEV device if SEV init fails > crypto: ccp: Detect and reject "invalid" addresses destined for PSP > crypto: ccp: Reject SEV commands with mismatching command buffer > crypto: ccp: Play nice with vmalloc'd memory for SEV command structs > crypto: ccp: Use the stack for small SEV command buffers > crypto: ccp: Use the stack and common buffer for status commands > crypto: ccp: Use the stack and common buffer for INIT command > KVM: SVM: Allocate SEV command structures on local stack > > arch/x86/kvm/svm/sev.c | 262 +++++++++++++---------------------- > drivers/crypto/ccp/sev-dev.c | 197 +++++++++++++------------- > drivers/crypto/ccp/sev-dev.h | 4 +- > 3 files changed, 196 insertions(+), 267 deletions(-) > Thanks Sean. Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
On 4/6/21 5:49 PM, Sean Christopherson wrote: > This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd > command buffers by copying _all_ incoming data pointers to an internal > buffer before sending the command to the PSP. The SEV driver and KVM are > then converted to use the stack for all command buffers. > > Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere > near enough about the PSP to give it the right input. > > v2: > - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs > sharing SEV context"). > - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh] > - Allocate a full page for the buffer. [Brijesh] > - Drop one set of the "!"s. [Christophe] > - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary > patch (definitely feel free to drop the patch if it's not worth > backporting). [Christophe] > - s/intput/input/. [Tom] > - Add a patch to free "sev" if init fails. This is not strictly > necessary (I think; I suck horribly when it comes to the driver > framework). But it felt wrong to not free cmd_buf on failure, and > even more wrong to free cmd_buf but not sev. > > v1: > - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20210402233702.3291792-1-seanjc%40google.com&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cecd38fba67954845323908d8f94e5405%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637533462102772796%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SUN8Zp%2Fi%2BiHAjMSe%2Fjwvs9JmXg%2FRvi%2B8j01sLDipPg8%3D&reserved=0 > > Sean Christopherson (8): > crypto: ccp: Free SEV device if SEV init fails > crypto: ccp: Detect and reject "invalid" addresses destined for PSP > crypto: ccp: Reject SEV commands with mismatching command buffer > crypto: ccp: Play nice with vmalloc'd memory for SEV command structs > crypto: ccp: Use the stack for small SEV command buffers > crypto: ccp: Use the stack and common buffer for status commands > crypto: ccp: Use the stack and common buffer for INIT command > KVM: SVM: Allocate SEV command structures on local stack > > arch/x86/kvm/svm/sev.c | 262 +++++++++++++---------------------- > drivers/crypto/ccp/sev-dev.c | 197 +++++++++++++------------- > drivers/crypto/ccp/sev-dev.h | 4 +- > 3 files changed, 196 insertions(+), 267 deletions(-) For the series: Acked-by: Tom Lendacky <thomas.lendacky@amd.com> >
On 07/04/21 20:00, Tom Lendacky wrote: > For the series: > > Acked-by: Tom Lendacky<thomas.lendacky@amd.com> Shall I take this as a request (or permission, whatever :)) to merge it through the KVM tree? Paolo
On 4/15/21 11:09 AM, Paolo Bonzini wrote: > On 07/04/21 20:00, Tom Lendacky wrote: >> For the series: >> >> Acked-by: Tom Lendacky<thomas.lendacky@amd.com> > > Shall I take this as a request (or permission, whatever :)) to merge it > through the KVM tree? Adding Herbert. Here's a link to the series: https://lore.kernel.org/kvm/88eef561-6fd8-a495-0d60-ff688070cc9e@redhat.com/T/#m2bbdd12452970d3bd7d0b1464c22bf2f0227a9f1 I'm not sure how you typically do the cross-tree stuff. Patch 8 has a requirement on patches 1-7. The arch/x86/kvm/svm/sev.c file tends to have more activity/changes than drivers/crypto/ccp/sev-dev.{c,h}, so it would make sense to take it through the KVM tree. But I think you need to verify that with Herbert. Thanks, Tom > > Paolo >
On Thu, Apr 15, 2021 at 01:15:59PM -0500, Tom Lendacky wrote: > On 4/15/21 11:09 AM, Paolo Bonzini wrote: > > On 07/04/21 20:00, Tom Lendacky wrote: > >> For the series: > >> > >> Acked-by: Tom Lendacky<thomas.lendacky@amd.com> > > > > Shall I take this as a request (or permission, whatever :)) to merge it > > through the KVM tree? > > Adding Herbert. Here's a link to the series: > > https://lore.kernel.org/kvm/88eef561-6fd8-a495-0d60-ff688070cc9e@redhat.com/T/#m2bbdd12452970d3bd7d0b1464c22bf2f0227a9f1 > > I'm not sure how you typically do the cross-tree stuff. Patch 8 has a > requirement on patches 1-7. The arch/x86/kvm/svm/sev.c file tends to have > more activity/changes than drivers/crypto/ccp/sev-dev.{c,h}, so it would > make sense to take it through the KVM tree. But I think you need to verify > that with Herbert. I don't mind at all. Paolo you can take this through your tree. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 07/04/21 07:20, Christophe Leroy wrote: >> >> + struct sev_data_init data; > > struct sev_data_init data = {0, 0, 0, 0}; Having to count the number of items is suboptimal. The alternative could be {} (which however is technically not standard C), {0} (a bit mysterious, but it works) and memset. I kept the latter to avoid touching the submitter's patch too much. Paolo
On 07/04/21 00:49, Sean Christopherson wrote: > This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd > command buffers by copying _all_ incoming data pointers to an internal > buffer before sending the command to the PSP. The SEV driver and KVM are > then converted to use the stack for all command buffers. > > Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere > near enough about the PSP to give it the right input. > > v2: > - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs > sharing SEV context"). > - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh] > - Allocate a full page for the buffer. [Brijesh] > - Drop one set of the "!"s. [Christophe] > - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary > patch (definitely feel free to drop the patch if it's not worth > backporting). [Christophe] > - s/intput/input/. [Tom] > - Add a patch to free "sev" if init fails. This is not strictly > necessary (I think; I suck horribly when it comes to the driver > framework). But it felt wrong to not free cmd_buf on failure, and > even more wrong to free cmd_buf but not sev. > > v1: > - https://lkml.kernel.org/r/20210402233702.3291792-1-seanjc@google.com > > Sean Christopherson (8): > crypto: ccp: Free SEV device if SEV init fails > crypto: ccp: Detect and reject "invalid" addresses destined for PSP > crypto: ccp: Reject SEV commands with mismatching command buffer > crypto: ccp: Play nice with vmalloc'd memory for SEV command structs > crypto: ccp: Use the stack for small SEV command buffers > crypto: ccp: Use the stack and common buffer for status commands > crypto: ccp: Use the stack and common buffer for INIT command > KVM: SVM: Allocate SEV command structures on local stack > > arch/x86/kvm/svm/sev.c | 262 +++++++++++++---------------------- > drivers/crypto/ccp/sev-dev.c | 197 +++++++++++++------------- > drivers/crypto/ccp/sev-dev.h | 4 +- > 3 files changed, 196 insertions(+), 267 deletions(-) > Queued, thanks. Paolo
This series teaches __sev_do_cmd_locked() to gracefully handle vmalloc'd command buffers by copying _all_ incoming data pointers to an internal buffer before sending the command to the PSP. The SEV driver and KVM are then converted to use the stack for all command buffers. Tested everything except sev_ioctl_do_pek_import(), I don't know anywhere near enough about the PSP to give it the right input. v2: - Rebase to kvm/queue, commit f96be2deac9b ("KVM: x86: Support KVM VMs sharing SEV context"). - Unconditionally copy @data to the internal buffer. [Christophe, Brijesh] - Allocate a full page for the buffer. [Brijesh] - Drop one set of the "!"s. [Christophe] - Use virt_addr_valid() instead of is_vmalloc_addr() for the temporary patch (definitely feel free to drop the patch if it's not worth backporting). [Christophe] - s/intput/input/. [Tom] - Add a patch to free "sev" if init fails. This is not strictly necessary (I think; I suck horribly when it comes to the driver framework). But it felt wrong to not free cmd_buf on failure, and even more wrong to free cmd_buf but not sev. v1: - https://lkml.kernel.org/r/20210402233702.3291792-1-seanjc@google.com Sean Christopherson (8): crypto: ccp: Free SEV device if SEV init fails crypto: ccp: Detect and reject "invalid" addresses destined for PSP crypto: ccp: Reject SEV commands with mismatching command buffer crypto: ccp: Play nice with vmalloc'd memory for SEV command structs crypto: ccp: Use the stack for small SEV command buffers crypto: ccp: Use the stack and common buffer for status commands crypto: ccp: Use the stack and common buffer for INIT command KVM: SVM: Allocate SEV command structures on local stack arch/x86/kvm/svm/sev.c | 262 +++++++++++++---------------------- drivers/crypto/ccp/sev-dev.c | 197 +++++++++++++------------- drivers/crypto/ccp/sev-dev.h | 4 +- 3 files changed, 196 insertions(+), 267 deletions(-)