[PATCH] virt: sev-guest: Satisfy linear mapping requirement in get_derived_key()

[PATCH] virt: sev-guest: Satisfy linear mapping requirement in get_derived_key()

[Tom Lendacky]

Commit 7ffeb2fc2670 ("x86/sev: Document requirement for linear mapping of
guest request buffers") added a check that requires the guest request
buffers to be in the linear mapping. The get_derived_key() function was
passing a buffer that was allocated on the stack, resulting in the call
to snp_send_guest_request() returning an error.

Update the get_derived_key() function to use an allocated buffer instead
of a stack buffer.

Fixes: 7ffeb2fc2670 ("x86/sev: Document requirement for linear mapping of guest request buffers")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/virt/coco/sev-guest/sev-guest.c | 27 +++++++++++--------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index d2b3ae7113ab..b01ec99106cd 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -116,13 +116,11 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
 
 static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
 {
+	struct snp_derived_key_resp *derived_key_resp __free(kfree) = NULL;
 	struct snp_derived_key_req *derived_key_req __free(kfree) = NULL;
-	struct snp_derived_key_resp derived_key_resp = {0};
 	struct snp_msg_desc *mdesc = snp_dev->msg_desc;
 	struct snp_guest_req req = {};
 	int rc, resp_len;
-	/* Response data is 64 bytes and max authsize for GCM is 16 bytes. */
-	u8 buf[64 + 16];
 
 	if (!arg->req_data || !arg->resp_data)
 		return -EINVAL;
@@ -132,8 +130,9 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
 	 * response payload. Make sure that it has enough space to cover the
 	 * authtag.
 	 */
-	resp_len = sizeof(derived_key_resp.data) + mdesc->ctx->authsize;
-	if (sizeof(buf) < resp_len)
+	resp_len = sizeof(derived_key_resp->data) + mdesc->ctx->authsize;
+	derived_key_resp = kzalloc(resp_len, GFP_KERNEL_ACCOUNT);
+	if (!derived_key_resp)
 		return -ENOMEM;
 
 	derived_key_req = kzalloc(sizeof(*derived_key_req), GFP_KERNEL_ACCOUNT);
@@ -149,23 +148,21 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
 	req.vmpck_id = mdesc->vmpck_id;
 	req.req_buf = derived_key_req;
 	req.req_sz = sizeof(*derived_key_req);
-	req.resp_buf = buf;
+	req.resp_buf = derived_key_resp;
 	req.resp_sz = resp_len;
 	req.exit_code = SVM_VMGEXIT_GUEST_REQUEST;
 
 	rc = snp_send_guest_request(mdesc, &req);
 	arg->exitinfo2 = req.exitinfo2;
-	if (rc)
-		return rc;
-
-	memcpy(derived_key_resp.data, buf, sizeof(derived_key_resp.data));
-	if (copy_to_user((void __user *)arg->resp_data, &derived_key_resp,
-			 sizeof(derived_key_resp)))
-		rc = -EFAULT;
+	if (!rc) {
+		if (copy_to_user((void __user *)arg->resp_data, derived_key_resp,
+				 sizeof(derived_key_resp->data)))
+			rc = -EFAULT;
+	}
 
 	/* The response buffer contains the sensitive data, explicitly clear it. */
-	memzero_explicit(buf, sizeof(buf));
-	memzero_explicit(&derived_key_resp, sizeof(derived_key_resp));
+	memzero_explicit(derived_key_resp, sizeof(*derived_key_resp));
+
 	return rc;
 }
 

base-commit: e180b3a224cb519388c2f61ca7bc1eaf94cec1fb
-- 
2.46.2

Re: [PATCH] virt: sev-guest: Satisfy linear mapping requirement in get_derived_key()

[Borislav Petkov]

On Wed, Jul 16, 2025 at 03:41:35PM -0500, Tom Lendacky wrote:
> @@ -149,23 +148,21 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
>  	req.vmpck_id = mdesc->vmpck_id;
>  	req.req_buf = derived_key_req;
>  	req.req_sz = sizeof(*derived_key_req);
> -	req.resp_buf = buf;
> +	req.resp_buf = derived_key_resp;
>  	req.resp_sz = resp_len;
>  	req.exit_code = SVM_VMGEXIT_GUEST_REQUEST;
>  
>  	rc = snp_send_guest_request(mdesc, &req);
>  	arg->exitinfo2 = req.exitinfo2;
> -	if (rc)
> -		return rc;
> -
> -	memcpy(derived_key_resp.data, buf, sizeof(derived_key_resp.data));
> -	if (copy_to_user((void __user *)arg->resp_data, &derived_key_resp,
> -			 sizeof(derived_key_resp)))
> -		rc = -EFAULT;
> +	if (!rc) {

Why is that rc test not right after the rc assignment?

> +		if (copy_to_user((void __user *)arg->resp_data, derived_key_resp,
> +				 sizeof(derived_key_resp->data)))
> +			rc = -EFAULT;
> +	}

I see a couple of places in the driver where this is done and that exitinfo2
thing is showed in-between.

/me goes and looks

Aha, the caller looks at it and copies it to luserspace.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette