| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <17e9b20d-fc69-8fed-e6cb-c30fa33aa9e7@amd.com>
Date: Mon, 24 Oct 2022 09:22:14 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Dionna Glaze <dionnaglaze@...gle.com>,
linux-kernel@...r.kernel.org, x86@...nel.org
Cc: Paolo Bonzini <pbonzini@...hat.com>,
Joerg Roedel <jroedel@...e.de>,
Peter Gonda <pgonda@...gle.com>,
Thomas Gleixner <tglx@...utronix.de>,
Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH v3 4/4] virt/coco/sev-guest: interpret VMM errors from
guest request
On 10/21/22 11:59, Dionna Glaze wrote:
> The GHCB specification states that the upper 32 bits of exitinfo2 are
> for the VMM's error codes. The sev-guest ABI has already locked in
> that the fw_err status of the input will be 64 bits, and that
> BIT_ULL(32) means that the extended guest request's data buffer was too
> small, so we have to keep that ABI.
>
> We can still interpret the upper 32 bits of exitinfo2 for the user
> anyway in case the request gets throttled. For safety, since the
> encryption algorithm in GHCBv2 is AES_GCM, we cannot return to user
> space without having completed the request with the current sequence
> number. If we were to return and the guest were to make another request
> but with different message contents, then that would be IV reuse.
>
> When throttled, the driver will reschedule itself and then try
> again. The ioctl may block indefinitely, but that has always been the
> case when deferring these requests to the host.
>
> Cc: Tom Lendacky <Thomas.Lendacky@....com>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Joerg Roedel <jroedel@...e.de>
> Cc: Peter Gonda <pgonda@...gle.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@...gle.com>
> ---
> drivers/virt/coco/sev-guest/sev-guest.c | 32 ++++++++++++++++++++-----
> include/uapi/linux/sev-guest.h | 18 ++++++++++++--
> 2 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 0508c2f46f6b..7abf4c3daa6d 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -305,9 +305,12 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
> u8 type, void *req_buf, size_t req_sz, void *resp_buf,
> u32 resp_sz, __u64 *exitinfo2)
> {
> + unsigned int vmm_err;
> u64 seqno;
> int rc;
>
> + might_resched();
> +
> /* Get message sequence and verify that its a non-zero */
> seqno = snp_get_msg_seqno(snp_dev);
> if (!seqno)
> @@ -320,9 +323,26 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
> if (rc)
> return rc;
>
> +retry:
> /* Call firmware to process the request */
> rc = snp_issue_guest_request(exit_code, &snp_dev->input, exitinfo2);
>
> + vmm_err = *exitinfo2 >> SNP_GUEST_VMM_ERR_SHIFT;
> + /*
> + * The host may return EBUSY if the request has been throttled.
> + * We retry in the driver to avoid returning and reusing the message
> + * sequence number on a different message.
> + */
> + if (vmm_err == SNP_GUEST_VMM_ERR_BUSY) {
> + cond_resched();
I would think there should be some form of delay here. Otherwise, it is
likely that the request could be re-issued almost immediately, which may
make the hypervisor think even more requests are being issued and try to
rate limit the guest even more.
Thanks,
Tom
> + goto retry;
> + }
> +
> + if (vmm_err && vmm_err != SNP_GUEST_VMM_ERR_INVALID_LEN) {
> + pr_err("sev-guest: host returned unknown error code: %d\n",
> + vmm_err);
> + return -EINVAL;
> + }
> if (rc)
> return rc;
>
> @@ -375,7 +395,7 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
>
> rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version,
> SNP_MSG_REPORT_REQ, &req, sizeof(req), resp->data,
> - resp_len, &arg->fw_err);
> + resp_len, &arg->exitinfo2);
> if (rc)
> goto e_free;
>
> @@ -415,7 +435,7 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
>
> rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg->msg_version,
> SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len,
> - &arg->fw_err);
> + &arg->exitinfo2);
> if (rc)
> return rc;
>
> @@ -477,10 +497,10 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> snp_dev->input.data_npages = npages;
> ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg->msg_version,
> SNP_MSG_REPORT_REQ, &req.data,
> - sizeof(req.data), resp->data, resp_len, &arg->fw_err);
> + sizeof(req.data), resp->data, resp_len, &arg->exitinfo2);
>
> /* If certs length is invalid then copy the returned length */
> - if (arg->fw_err == SNP_GUEST_REQ_INVALID_LEN) {
> + if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
> req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT;
>
> if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req)))
> @@ -515,7 +535,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
> if (copy_from_user(&input, argp, sizeof(input)))
> return -EFAULT;
>
> - input.fw_err = 0xff;
> + input.exitinfo2 = SEV_RET_NO_FW_CALL;
>
> /* Message version must be non-zero */
> if (!input.msg_version)
> @@ -546,7 +566,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
>
> mutex_unlock(&snp_cmd_mutex);
>
> - if (input.fw_err && copy_to_user(argp, &input, sizeof(input)))
> + if (input.exitinfo2 && copy_to_user(argp, &input, sizeof(input)))
> return -EFAULT;
>
> return ret;
> diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h
> index 256aaeff7e65..8e4144aa78c9 100644
> --- a/include/uapi/linux/sev-guest.h
> +++ b/include/uapi/linux/sev-guest.h
> @@ -52,8 +52,15 @@ struct snp_guest_request_ioctl {
> __u64 req_data;
> __u64 resp_data;
>
> - /* firmware error code on failure (see psp-sev.h) */
> - __u64 fw_err;
> + /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */
> + union {
> + __u64 exitinfo2;
> + __u64 fw_err; /* Name deprecated in favor of others */
> + struct {
> + __u32 fw_error;
> + __u32 vmm_error;
> + };
> + };
> };
>
> struct snp_ext_report_req {
> @@ -77,4 +84,11 @@ struct snp_ext_report_req {
> /* Get SNP extended report as defined in the GHCB specification version 2. */
> #define SNP_GET_EXT_REPORT _IOWR(SNP_GUEST_REQ_IOC_TYPE, 0x2, struct snp_guest_request_ioctl)
>
> +/* Guest message request EXIT_INFO_2 constants */
> +#define SNP_GUEST_FW_ERR_MASK GENMASK_ULL(31, 0)
> +#define SNP_GUEST_VMM_ERR_SHIFT 32
> +
> +#define SNP_GUEST_VMM_ERR_INVALID_LEN 1
> +#define SNP_GUEST_VMM_ERR_BUSY 2
> +
> #endif /* __UAPI_LINUX_SEV_GUEST_H_ */
Powered by blists - more mailing lists