Messages in this thread | | | Subject | Re: [PATCH v7 44/45] virt: sevguest: Add support to derive key | From | Brijesh Singh <> | Date | Thu, 18 Nov 2021 11:43:23 -0600 |
| |
On 11/18/21 10:43 AM, Peter Gonda wrote: ... >> + u8 buf[89]; > > Could we document this magic number? >
Yes, I will document from where this number came.
>> + >> + if (!arg->req_data || !arg->resp_data) >> + return -EINVAL; >> + >> + /* Copy the request payload from userspace */ >> + if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req))) >> + return -EFAULT; >> + >> + /* Message version must be non-zero */ >> + if (!req.msg_version) >> + return -EINVAL; >> + >> + /* >> + * The intermediate response buffer is used while decrypting the >> + * response payload. Make sure that it has enough space to cover the >> + * authtag. >> + */ >> + resp_len = sizeof(resp.data) + crypto->a_len; >> + if (sizeof(buf) < resp_len) >> + return -ENOMEM; >> + >> + /* Issue the command to get the attestation report */ >> + rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, req.msg_version, >> + SNP_MSG_KEY_REQ, &req.data, sizeof(req.data), buf, resp_len, >> + &arg->fw_err); >> + if (rc) >> + goto e_free; > > Should we check the first 32 bits of |data| here since that is a > status field? If we see 16h here we could return -EINVAL, or better to > let userspace deal with that error handling? >
I was trying to avoid looking into a response structure to keep the flexibility; e.g if SNP firmware changes a response format then we don't need to have any changes in the driver. The userspace should be able to deal with it and it can check the "status" or a new field etc.
thanks
| |