* FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree
@ 2025-03-09 10:29 gregkh
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: gregkh @ 2025-03-09 10:29 UTC (permalink / raw)
To: aik, bp, nikunj; +Cc: stable
The patch below does not apply to the 6.13-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.13.y
git checkout FETCH_HEAD
git cherry-pick -x 3e385c0d6ce88ac9916dcf84267bd5855d830748
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2025030957-magnetism-lustily-55d9@gregkh' --subject-prefix 'PATCH 6.13.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 3e385c0d6ce88ac9916dcf84267bd5855d830748 Mon Sep 17 00:00:00 2001
From: Alexey Kardashevskiy <aik@amd.com>
Date: Fri, 7 Mar 2025 12:37:00 +1100
Subject: [PATCH] virt: sev-guest: Move SNP Guest Request data pages handling
under snp_cmd_mutex
Compared to the SNP Guest Request, the "Extended" version adds data pages for
receiving certificates. If not enough pages provided, the HV can report to the
VM how much is needed so the VM can reallocate and repeat.
Commit
ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
moved handling of the allocated/desired pages number out of scope of said
mutex and create a possibility for a race (multiple instances trying to
trigger Extended request in a VM) as there is just one instance of
snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
Fix the issue by moving the data blob/size and the GHCB input struct
(snp_req_data) into snp_guest_req which is allocated on stack now and accessed
by the GHCB caller under that mutex.
Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of four
callers needs it. Free the received blob in get_ext_report() right after it is
copied to the userspace. Possible future users of snp_send_guest_request() are
likely to have different ideas about the buffer size anyways.
Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@amd.com
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 82492efc5d94..96c7bc698e6b 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2853,19 +2853,8 @@ struct snp_msg_desc *snp_msg_alloc(void)
if (!mdesc->response)
goto e_free_request;
- mdesc->certs_data = alloc_shared_pages(SEV_FW_BLOB_MAX_SIZE);
- if (!mdesc->certs_data)
- goto e_free_response;
-
- /* initial the input address for guest request */
- mdesc->input.req_gpa = __pa(mdesc->request);
- mdesc->input.resp_gpa = __pa(mdesc->response);
- mdesc->input.data_gpa = __pa(mdesc->certs_data);
-
return mdesc;
-e_free_response:
- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
e_free_request:
free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
e_unmap:
@@ -2885,7 +2874,6 @@ void snp_msg_free(struct snp_msg_desc *mdesc)
kfree(mdesc->ctx);
free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
iounmap((__force void __iomem *)mdesc->secrets);
memset(mdesc, 0, sizeof(*mdesc));
@@ -3054,7 +3042,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
- rc = snp_issue_guest_request(req, &mdesc->input, rio);
+ rc = snp_issue_guest_request(req, &req->input, rio);
switch (rc) {
case -ENOSPC:
/*
@@ -3064,7 +3052,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* order to increment the sequence number and thus avoid
* IV reuse.
*/
- override_npages = mdesc->input.data_npages;
+ override_npages = req->input.data_npages;
req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
/*
@@ -3120,7 +3108,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
}
if (override_npages)
- mdesc->input.data_npages = override_npages;
+ req->input.data_npages = override_npages;
return rc;
}
@@ -3158,6 +3146,11 @@ int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
*/
memcpy(mdesc->request, &mdesc->secret_request, sizeof(mdesc->secret_request));
+ /* Initialize the input address for guest request */
+ req->input.req_gpa = __pa(mdesc->request);
+ req->input.resp_gpa = __pa(mdesc->response);
+ req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
+
rc = __handle_guest_request(mdesc, req, rio);
if (rc) {
if (rc == -EIO &&
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 1581246491b5..ba7999f66abe 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -203,6 +203,9 @@ struct snp_guest_req {
unsigned int vmpck_id;
u8 msg_version;
u8 msg_type;
+
+ struct snp_req_data input;
+ void *certs_data;
};
/*
@@ -263,9 +266,6 @@ struct snp_msg_desc {
struct snp_guest_msg secret_request, secret_response;
struct snp_secrets_page *secrets;
- struct snp_req_data input;
-
- void *certs_data;
struct aesgcm_ctx *ctx;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 23ac177472be..70fbc9a3e703 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -176,6 +176,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
struct snp_guest_req req = {};
int ret, npages = 0, resp_len;
sockptr_t certs_address;
+ struct page *page;
if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
return -EINVAL;
@@ -209,8 +210,20 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
* the host. If host does not supply any certs in it, then copy
* zeros to indicate that certificate data was not provided.
*/
- memset(mdesc->certs_data, 0, report_req->certs_len);
npages = report_req->certs_len >> PAGE_SHIFT;
+ page = alloc_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
+ get_order(report_req->certs_len));
+ if (!page)
+ return -ENOMEM;
+
+ req.certs_data = page_address(page);
+ ret = set_memory_decrypted((unsigned long)req.certs_data, npages);
+ if (ret) {
+ pr_err("failed to mark page shared, ret=%d\n", ret);
+ __free_pages(page, get_order(report_req->certs_len));
+ return -EFAULT;
+ }
+
cmd:
/*
* The intermediate response buffer is used while decrypting the
@@ -219,10 +232,12 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
*/
resp_len = sizeof(report_resp->data) + mdesc->ctx->authsize;
report_resp = kzalloc(resp_len, GFP_KERNEL_ACCOUNT);
- if (!report_resp)
- return -ENOMEM;
+ if (!report_resp) {
+ ret = -ENOMEM;
+ goto e_free_data;
+ }
- mdesc->input.data_npages = npages;
+ req.input.data_npages = npages;
req.msg_version = arg->msg_version;
req.msg_type = SNP_MSG_REPORT_REQ;
@@ -237,7 +252,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
/* If certs length is invalid then copy the returned length */
if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
- report_req->certs_len = mdesc->input.data_npages << PAGE_SHIFT;
+ report_req->certs_len = req.input.data_npages << PAGE_SHIFT;
if (copy_to_sockptr(io->req_data, report_req, sizeof(*report_req)))
ret = -EFAULT;
@@ -246,7 +261,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
if (ret)
goto e_free;
- if (npages && copy_to_sockptr(certs_address, mdesc->certs_data, report_req->certs_len)) {
+ if (npages && copy_to_sockptr(certs_address, req.certs_data, report_req->certs_len)) {
ret = -EFAULT;
goto e_free;
}
@@ -256,6 +271,13 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
e_free:
kfree(report_resp);
+e_free_data:
+ if (npages) {
+ if (set_memory_encrypted((unsigned long)req.certs_data, npages))
+ WARN_ONCE(ret, "failed to restore encryption mask (leak it)\n");
+ else
+ __free_pages(page, get_order(report_req->certs_len));
+ }
return ret;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-09 10:29 FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree gregkh
@ 2025-03-10 10:00 ` Alexey Kardashevskiy
2025-03-10 10:13 ` Alexey Kardashevskiy
2025-03-13 9:01 ` Sasha Levin
2025-03-10 15:35 ` FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree Borislav Petkov
2025-03-11 7:36 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2 siblings, 2 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2025-03-10 10:00 UTC (permalink / raw)
To: stable; +Cc: Alexey Kardashevskiy, Nikunj A Dadhania
Compared to the SNP Guest Request, the "Extended" version adds data pages
for receiving certificates. If not enough pages provided, the HV can
report to the VM how much is needed so the VM can reallocate and repeat.
Commit ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command
mutex") moved handling of the allocated/desired pages number out of scope
of said mutex and create a possibility for a race (multiple instances
trying to trigger Extended request in a VM) as there is just one instance
of snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
Fix the issue by moving the data blob/size and the GHCB input struct
(snp_req_data) into snp_guest_req which is allocated on stack now
and accessed by the GHCB caller under that mutex.
Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of
four callers needs it. Free the received blob in get_ext_report() right
after it is copied to the userspace. Possible future users of
snp_send_guest_request() are likely to have different ideas about
the buffer size anyways.
Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
Cc: stable@vger.kernel.org # 6.13
Cc: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
arch/x86/include/asm/sev.h | 6 +--
drivers/virt/coco/sev-guest/sev-guest.c | 63 +++++++++++++++----------
2 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 91f08af31078..82d9250aac34 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -185,6 +185,9 @@ struct snp_guest_req {
unsigned int vmpck_id;
u8 msg_version;
u8 msg_type;
+
+ struct snp_req_data input;
+ void *certs_data;
};
/*
@@ -245,9 +248,6 @@ struct snp_msg_desc {
struct snp_guest_msg secret_request, secret_response;
struct snp_secrets_page *secrets;
- struct snp_req_data input;
-
- void *certs_data;
struct aesgcm_ctx *ctx;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index af64e6191f74..480159606434 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -249,7 +249,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
- rc = snp_issue_guest_request(req, &mdesc->input, rio);
+ rc = snp_issue_guest_request(req, &req->input, rio);
switch (rc) {
case -ENOSPC:
/*
@@ -259,7 +259,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* order to increment the sequence number and thus avoid
* IV reuse.
*/
- override_npages = mdesc->input.data_npages;
+ override_npages = req->input.data_npages;
req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
/*
@@ -315,7 +315,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
}
if (override_npages)
- mdesc->input.data_npages = override_npages;
+ req->input.data_npages = override_npages;
return rc;
}
@@ -354,6 +354,11 @@ static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
memcpy(mdesc->request, &mdesc->secret_request,
sizeof(mdesc->secret_request));
+ /* initial the input address for guest request */
+ req->input.req_gpa = __pa(mdesc->request);
+ req->input.resp_gpa = __pa(mdesc->response);
+ req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
+
rc = __handle_guest_request(mdesc, req, rio);
if (rc) {
if (rc == -EIO &&
@@ -495,6 +500,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
struct snp_guest_req req = {};
int ret, npages = 0, resp_len;
sockptr_t certs_address;
+ struct page *page;
if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
return -EINVAL;
@@ -528,8 +534,20 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
* the host. If host does not supply any certs in it, then copy
* zeros to indicate that certificate data was not provided.
*/
- memset(mdesc->certs_data, 0, report_req->certs_len);
npages = report_req->certs_len >> PAGE_SHIFT;
+ page = alloc_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
+ get_order(report_req->certs_len));
+ if (!page)
+ return -ENOMEM;
+
+ req.certs_data = page_address(page);
+ ret = set_memory_decrypted((unsigned long)req.certs_data, npages);
+ if (ret) {
+ pr_err("failed to mark page shared, ret=%d\n", ret);
+ __free_pages(page, get_order(report_req->certs_len));
+ return -EFAULT;
+ }
+
cmd:
/*
* The intermediate response buffer is used while decrypting the
@@ -538,10 +556,12 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
*/
resp_len = sizeof(report_resp->data) + mdesc->ctx->authsize;
report_resp = kzalloc(resp_len, GFP_KERNEL_ACCOUNT);
- if (!report_resp)
- return -ENOMEM;
+ if (!report_resp) {
+ ret = -ENOMEM;
+ goto e_free_data;
+ }
- mdesc->input.data_npages = npages;
+ req.input.data_npages = npages;
req.msg_version = arg->msg_version;
req.msg_type = SNP_MSG_REPORT_REQ;
@@ -556,7 +576,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
/* If certs length is invalid then copy the returned length */
if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
- report_req->certs_len = mdesc->input.data_npages << PAGE_SHIFT;
+ report_req->certs_len = req.input.data_npages << PAGE_SHIFT;
if (copy_to_sockptr(io->req_data, report_req, sizeof(*report_req)))
ret = -EFAULT;
@@ -565,7 +585,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
if (ret)
goto e_free;
- if (npages && copy_to_sockptr(certs_address, mdesc->certs_data, report_req->certs_len)) {
+ if (npages && copy_to_sockptr(certs_address, req.certs_data, report_req->certs_len)) {
ret = -EFAULT;
goto e_free;
}
@@ -575,6 +595,13 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
e_free:
kfree(report_resp);
+e_free_data:
+ if (npages) {
+ if (set_memory_encrypted((unsigned long)req.certs_data, npages))
+ WARN_ONCE(ret, "failed to restore encryption mask (leak it)\n");
+ else
+ __free_pages(page, get_order(report_req->certs_len));
+ }
return ret;
}
@@ -1048,35 +1075,26 @@ static int __init sev_guest_probe(struct platform_device *pdev)
if (!mdesc->response)
goto e_free_request;
- mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
- if (!mdesc->certs_data)
- goto e_free_response;
-
ret = -EIO;
mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
if (!mdesc->ctx)
- goto e_free_cert_data;
+ goto e_free_response;
misc = &snp_dev->misc;
misc->minor = MISC_DYNAMIC_MINOR;
misc->name = DEVICE_NAME;
misc->fops = &snp_guest_fops;
- /* Initialize the input addresses for guest request */
- mdesc->input.req_gpa = __pa(mdesc->request);
- mdesc->input.resp_gpa = __pa(mdesc->response);
- mdesc->input.data_gpa = __pa(mdesc->certs_data);
-
/* Set the privlevel_floor attribute based on the vmpck_id */
sev_tsm_ops.privlevel_floor = vmpck_id;
ret = tsm_register(&sev_tsm_ops, snp_dev);
if (ret)
- goto e_free_cert_data;
+ goto e_free_response;
ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
if (ret)
- goto e_free_cert_data;
+ goto e_free_response;
ret = misc_register(misc);
if (ret)
@@ -1088,8 +1106,6 @@ static int __init sev_guest_probe(struct platform_device *pdev)
e_free_ctx:
kfree(mdesc->ctx);
-e_free_cert_data:
- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
e_free_response:
free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
e_free_request:
@@ -1104,7 +1120,6 @@ static void __exit sev_guest_remove(struct platform_device *pdev)
struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
struct snp_msg_desc *mdesc = snp_dev->msg_desc;
- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
kfree(mdesc->ctx);
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
@ 2025-03-10 10:13 ` Alexey Kardashevskiy
2025-03-10 10:35 ` Greg KH
2025-03-13 9:01 ` Sasha Levin
1 sibling, 1 reply; 11+ messages in thread
From: Alexey Kardashevskiy @ 2025-03-10 10:13 UTC (permalink / raw)
To: stable; +Cc: Nikunj A Dadhania
On 10/3/25 21:00, Alexey Kardashevskiy wrote:
> Compared to the SNP Guest Request, the "Extended" version adds data pages
> for receiving certificates. If not enough pages provided, the HV can
> report to the VM how much is needed so the VM can reallocate and repeat.
>
> Commit ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command
> mutex") moved handling of the allocated/desired pages number out of scope
> of said mutex and create a possibility for a race (multiple instances
> trying to trigger Extended request in a VM) as there is just one instance
> of snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
>
> Fix the issue by moving the data blob/size and the GHCB input struct
> (snp_req_data) into snp_guest_req which is allocated on stack now
> and accessed by the GHCB caller under that mutex.
>
> Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of
> four callers needs it. Free the received blob in get_ext_report() right
> after it is copied to the userspace. Possible future users of
> snp_send_guest_request() are likely to have different ideas about
> the buffer size anyways.
>
> Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
> Cc: stable@vger.kernel.org # 6.13
> Cc: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Missed:
(cherry picked from commit 3e385c0d6ce88ac9916dcf84267bd5855d830748)
I first cherrypicked and sent, then I read about "cherry-oick -x", sorry
for the noise. thanks,
> ---
> arch/x86/include/asm/sev.h | 6 +--
> drivers/virt/coco/sev-guest/sev-guest.c | 63 +++++++++++++++----------
> 2 files changed, 42 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 91f08af31078..82d9250aac34 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -185,6 +185,9 @@ struct snp_guest_req {
> unsigned int vmpck_id;
> u8 msg_version;
> u8 msg_type;
> +
> + struct snp_req_data input;
> + void *certs_data;
> };
>
> /*
> @@ -245,9 +248,6 @@ struct snp_msg_desc {
> struct snp_guest_msg secret_request, secret_response;
>
> struct snp_secrets_page *secrets;
> - struct snp_req_data input;
> -
> - void *certs_data;
>
> struct aesgcm_ctx *ctx;
>
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index af64e6191f74..480159606434 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -249,7 +249,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> * sequence number must be incremented or the VMPCK must be deleted to
> * prevent reuse of the IV.
> */
> - rc = snp_issue_guest_request(req, &mdesc->input, rio);
> + rc = snp_issue_guest_request(req, &req->input, rio);
> switch (rc) {
> case -ENOSPC:
> /*
> @@ -259,7 +259,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> * order to increment the sequence number and thus avoid
> * IV reuse.
> */
> - override_npages = mdesc->input.data_npages;
> + override_npages = req->input.data_npages;
> req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
>
> /*
> @@ -315,7 +315,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> }
>
> if (override_npages)
> - mdesc->input.data_npages = override_npages;
> + req->input.data_npages = override_npages;
>
> return rc;
> }
> @@ -354,6 +354,11 @@ static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> memcpy(mdesc->request, &mdesc->secret_request,
> sizeof(mdesc->secret_request));
>
> + /* initial the input address for guest request */
> + req->input.req_gpa = __pa(mdesc->request);
> + req->input.resp_gpa = __pa(mdesc->response);
> + req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
> +
> rc = __handle_guest_request(mdesc, req, rio);
> if (rc) {
> if (rc == -EIO &&
> @@ -495,6 +500,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> struct snp_guest_req req = {};
> int ret, npages = 0, resp_len;
> sockptr_t certs_address;
> + struct page *page;
>
> if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
> return -EINVAL;
> @@ -528,8 +534,20 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> * the host. If host does not supply any certs in it, then copy
> * zeros to indicate that certificate data was not provided.
> */
> - memset(mdesc->certs_data, 0, report_req->certs_len);
> npages = report_req->certs_len >> PAGE_SHIFT;
> + page = alloc_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
> + get_order(report_req->certs_len));
> + if (!page)
> + return -ENOMEM;
> +
> + req.certs_data = page_address(page);
> + ret = set_memory_decrypted((unsigned long)req.certs_data, npages);
> + if (ret) {
> + pr_err("failed to mark page shared, ret=%d\n", ret);
> + __free_pages(page, get_order(report_req->certs_len));
> + return -EFAULT;
> + }
> +
> cmd:
> /*
> * The intermediate response buffer is used while decrypting the
> @@ -538,10 +556,12 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> */
> resp_len = sizeof(report_resp->data) + mdesc->ctx->authsize;
> report_resp = kzalloc(resp_len, GFP_KERNEL_ACCOUNT);
> - if (!report_resp)
> - return -ENOMEM;
> + if (!report_resp) {
> + ret = -ENOMEM;
> + goto e_free_data;
> + }
>
> - mdesc->input.data_npages = npages;
> + req.input.data_npages = npages;
>
> req.msg_version = arg->msg_version;
> req.msg_type = SNP_MSG_REPORT_REQ;
> @@ -556,7 +576,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
>
> /* If certs length is invalid then copy the returned length */
> if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
> - report_req->certs_len = mdesc->input.data_npages << PAGE_SHIFT;
> + report_req->certs_len = req.input.data_npages << PAGE_SHIFT;
>
> if (copy_to_sockptr(io->req_data, report_req, sizeof(*report_req)))
> ret = -EFAULT;
> @@ -565,7 +585,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> if (ret)
> goto e_free;
>
> - if (npages && copy_to_sockptr(certs_address, mdesc->certs_data, report_req->certs_len)) {
> + if (npages && copy_to_sockptr(certs_address, req.certs_data, report_req->certs_len)) {
> ret = -EFAULT;
> goto e_free;
> }
> @@ -575,6 +595,13 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
>
> e_free:
> kfree(report_resp);
> +e_free_data:
> + if (npages) {
> + if (set_memory_encrypted((unsigned long)req.certs_data, npages))
> + WARN_ONCE(ret, "failed to restore encryption mask (leak it)\n");
> + else
> + __free_pages(page, get_order(report_req->certs_len));
> + }
> return ret;
> }
>
> @@ -1048,35 +1075,26 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> if (!mdesc->response)
> goto e_free_request;
>
> - mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
> - if (!mdesc->certs_data)
> - goto e_free_response;
> -
> ret = -EIO;
> mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
> if (!mdesc->ctx)
> - goto e_free_cert_data;
> + goto e_free_response;
>
> misc = &snp_dev->misc;
> misc->minor = MISC_DYNAMIC_MINOR;
> misc->name = DEVICE_NAME;
> misc->fops = &snp_guest_fops;
>
> - /* Initialize the input addresses for guest request */
> - mdesc->input.req_gpa = __pa(mdesc->request);
> - mdesc->input.resp_gpa = __pa(mdesc->response);
> - mdesc->input.data_gpa = __pa(mdesc->certs_data);
> -
> /* Set the privlevel_floor attribute based on the vmpck_id */
> sev_tsm_ops.privlevel_floor = vmpck_id;
>
> ret = tsm_register(&sev_tsm_ops, snp_dev);
> if (ret)
> - goto e_free_cert_data;
> + goto e_free_response;
>
> ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
> if (ret)
> - goto e_free_cert_data;
> + goto e_free_response;
>
> ret = misc_register(misc);
> if (ret)
> @@ -1088,8 +1106,6 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>
> e_free_ctx:
> kfree(mdesc->ctx);
> -e_free_cert_data:
> - free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
> e_free_response:
> free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> e_free_request:
> @@ -1104,7 +1120,6 @@ static void __exit sev_guest_remove(struct platform_device *pdev)
> struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
> struct snp_msg_desc *mdesc = snp_dev->msg_desc;
>
> - free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
> free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
> kfree(mdesc->ctx);
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-10 10:13 ` Alexey Kardashevskiy
@ 2025-03-10 10:35 ` Greg KH
0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2025-03-10 10:35 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: stable, Nikunj A Dadhania
On Mon, Mar 10, 2025 at 09:13:11PM +1100, Alexey Kardashevskiy wrote:
>
>
> On 10/3/25 21:00, Alexey Kardashevskiy wrote:
> > Compared to the SNP Guest Request, the "Extended" version adds data pages
> > for receiving certificates. If not enough pages provided, the HV can
> > report to the VM how much is needed so the VM can reallocate and repeat.
> >
> > Commit ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command
> > mutex") moved handling of the allocated/desired pages number out of scope
> > of said mutex and create a possibility for a race (multiple instances
> > trying to trigger Extended request in a VM) as there is just one instance
> > of snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
> >
> > Fix the issue by moving the data blob/size and the GHCB input struct
> > (snp_req_data) into snp_guest_req which is allocated on stack now
> > and accessed by the GHCB caller under that mutex.
> >
> > Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of
> > four callers needs it. Free the received blob in get_ext_report() right
> > after it is copied to the userspace. Possible future users of
> > snp_send_guest_request() are likely to have different ideas about
> > the buffer size anyways.
> >
> > Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
> > Cc: stable@vger.kernel.org # 6.13
> > Cc: Nikunj A Dadhania <nikunj@amd.com>
> > Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
>
> Missed:
>
> (cherry picked from commit 3e385c0d6ce88ac9916dcf84267bd5855d830748)
>
> I first cherrypicked and sent, then I read about "cherry-oick -x", sorry for
> the noise. thanks,
Please resend with this in the commit so that our tools pick it up
properly.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree
2025-03-09 10:29 FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree gregkh
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
@ 2025-03-10 15:35 ` Borislav Petkov
2025-03-11 7:36 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2 siblings, 0 replies; 11+ messages in thread
From: Borislav Petkov @ 2025-03-10 15:35 UTC (permalink / raw)
To: gregkh; +Cc: aik, nikunj, stable
On Sun, Mar 09, 2025 at 11:29:57AM +0100, gregkh@linuxfoundation.org wrote:
>
> The patch below does not apply to the 6.13-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> To reproduce the conflict and resubmit, you may use the following commands:
>
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.13.y
> git checkout FETCH_HEAD
> git cherry-pick -x 3e385c0d6ce88ac9916dcf84267bd5855d830748
> # <resolve conflicts, build, test, etc.>
> git commit -s
> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2025030957-magnetism-lustily-55d9@gregkh' --subject-prefix 'PATCH 6.13.y' HEAD^..
>
> Possible dependencies:
Yeah, aik is going to send you a proper backport here.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-09 10:29 FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree gregkh
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2025-03-10 15:35 ` FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree Borislav Petkov
@ 2025-03-11 7:36 ` Alexey Kardashevskiy
2025-03-13 9:08 ` Sasha Levin
2 siblings, 1 reply; 11+ messages in thread
From: Alexey Kardashevskiy @ 2025-03-11 7:36 UTC (permalink / raw)
To: stable; +Cc: Alexey Kardashevskiy, Borislav Petkov (AMD), Nikunj A Dadhania
Compared to the SNP Guest Request, the "Extended" version adds data pages for
receiving certificates. If not enough pages provided, the HV can report to the
VM how much is needed so the VM can reallocate and repeat.
Commit
ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
moved handling of the allocated/desired pages number out of scope of said
mutex and create a possibility for a race (multiple instances trying to
trigger Extended request in a VM) as there is just one instance of
snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
Fix the issue by moving the data blob/size and the GHCB input struct
(snp_req_data) into snp_guest_req which is allocated on stack now and accessed
by the GHCB caller under that mutex.
Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of four
callers needs it. Free the received blob in get_ext_report() right after it is
copied to the userspace. Possible future users of snp_send_guest_request() are
likely to have different ideas about the buffer size anyways.
Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Cc: stable@vger.kernel.org # 6.13
Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@amd.com
(cherry picked from commit 3e385c0d6ce88ac9916dcf84267bd5855d830748)
---
arch/x86/include/asm/sev.h | 6 +--
drivers/virt/coco/sev-guest/sev-guest.c | 63 +++++++++++++++----------
2 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 91f08af31078..82d9250aac34 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -185,6 +185,9 @@ struct snp_guest_req {
unsigned int vmpck_id;
u8 msg_version;
u8 msg_type;
+
+ struct snp_req_data input;
+ void *certs_data;
};
/*
@@ -245,9 +248,6 @@ struct snp_msg_desc {
struct snp_guest_msg secret_request, secret_response;
struct snp_secrets_page *secrets;
- struct snp_req_data input;
-
- void *certs_data;
struct aesgcm_ctx *ctx;
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index af64e6191f74..480159606434 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -249,7 +249,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
- rc = snp_issue_guest_request(req, &mdesc->input, rio);
+ rc = snp_issue_guest_request(req, &req->input, rio);
switch (rc) {
case -ENOSPC:
/*
@@ -259,7 +259,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* order to increment the sequence number and thus avoid
* IV reuse.
*/
- override_npages = mdesc->input.data_npages;
+ override_npages = req->input.data_npages;
req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
/*
@@ -315,7 +315,7 @@ static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
}
if (override_npages)
- mdesc->input.data_npages = override_npages;
+ req->input.data_npages = override_npages;
return rc;
}
@@ -354,6 +354,11 @@ static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
memcpy(mdesc->request, &mdesc->secret_request,
sizeof(mdesc->secret_request));
+ /* initial the input address for guest request */
+ req->input.req_gpa = __pa(mdesc->request);
+ req->input.resp_gpa = __pa(mdesc->response);
+ req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
+
rc = __handle_guest_request(mdesc, req, rio);
if (rc) {
if (rc == -EIO &&
@@ -495,6 +500,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
struct snp_guest_req req = {};
int ret, npages = 0, resp_len;
sockptr_t certs_address;
+ struct page *page;
if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
return -EINVAL;
@@ -528,8 +534,20 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
* the host. If host does not supply any certs in it, then copy
* zeros to indicate that certificate data was not provided.
*/
- memset(mdesc->certs_data, 0, report_req->certs_len);
npages = report_req->certs_len >> PAGE_SHIFT;
+ page = alloc_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
+ get_order(report_req->certs_len));
+ if (!page)
+ return -ENOMEM;
+
+ req.certs_data = page_address(page);
+ ret = set_memory_decrypted((unsigned long)req.certs_data, npages);
+ if (ret) {
+ pr_err("failed to mark page shared, ret=%d\n", ret);
+ __free_pages(page, get_order(report_req->certs_len));
+ return -EFAULT;
+ }
+
cmd:
/*
* The intermediate response buffer is used while decrypting the
@@ -538,10 +556,12 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
*/
resp_len = sizeof(report_resp->data) + mdesc->ctx->authsize;
report_resp = kzalloc(resp_len, GFP_KERNEL_ACCOUNT);
- if (!report_resp)
- return -ENOMEM;
+ if (!report_resp) {
+ ret = -ENOMEM;
+ goto e_free_data;
+ }
- mdesc->input.data_npages = npages;
+ req.input.data_npages = npages;
req.msg_version = arg->msg_version;
req.msg_type = SNP_MSG_REPORT_REQ;
@@ -556,7 +576,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
/* If certs length is invalid then copy the returned length */
if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
- report_req->certs_len = mdesc->input.data_npages << PAGE_SHIFT;
+ report_req->certs_len = req.input.data_npages << PAGE_SHIFT;
if (copy_to_sockptr(io->req_data, report_req, sizeof(*report_req)))
ret = -EFAULT;
@@ -565,7 +585,7 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
if (ret)
goto e_free;
- if (npages && copy_to_sockptr(certs_address, mdesc->certs_data, report_req->certs_len)) {
+ if (npages && copy_to_sockptr(certs_address, req.certs_data, report_req->certs_len)) {
ret = -EFAULT;
goto e_free;
}
@@ -575,6 +595,13 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
e_free:
kfree(report_resp);
+e_free_data:
+ if (npages) {
+ if (set_memory_encrypted((unsigned long)req.certs_data, npages))
+ WARN_ONCE(ret, "failed to restore encryption mask (leak it)\n");
+ else
+ __free_pages(page, get_order(report_req->certs_len));
+ }
return ret;
}
@@ -1048,35 +1075,26 @@ static int __init sev_guest_probe(struct platform_device *pdev)
if (!mdesc->response)
goto e_free_request;
- mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
- if (!mdesc->certs_data)
- goto e_free_response;
-
ret = -EIO;
mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
if (!mdesc->ctx)
- goto e_free_cert_data;
+ goto e_free_response;
misc = &snp_dev->misc;
misc->minor = MISC_DYNAMIC_MINOR;
misc->name = DEVICE_NAME;
misc->fops = &snp_guest_fops;
- /* Initialize the input addresses for guest request */
- mdesc->input.req_gpa = __pa(mdesc->request);
- mdesc->input.resp_gpa = __pa(mdesc->response);
- mdesc->input.data_gpa = __pa(mdesc->certs_data);
-
/* Set the privlevel_floor attribute based on the vmpck_id */
sev_tsm_ops.privlevel_floor = vmpck_id;
ret = tsm_register(&sev_tsm_ops, snp_dev);
if (ret)
- goto e_free_cert_data;
+ goto e_free_response;
ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
if (ret)
- goto e_free_cert_data;
+ goto e_free_response;
ret = misc_register(misc);
if (ret)
@@ -1088,8 +1106,6 @@ static int __init sev_guest_probe(struct platform_device *pdev)
e_free_ctx:
kfree(mdesc->ctx);
-e_free_cert_data:
- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
e_free_response:
free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
e_free_request:
@@ -1104,7 +1120,6 @@ static void __exit sev_guest_remove(struct platform_device *pdev)
struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
struct snp_msg_desc *mdesc = snp_dev->msg_desc;
- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
kfree(mdesc->ctx);
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2025-03-10 10:13 ` Alexey Kardashevskiy
@ 2025-03-13 9:01 ` Sasha Levin
1 sibling, 0 replies; 11+ messages in thread
From: Sasha Levin @ 2025-03-13 9:01 UTC (permalink / raw)
To: stable, aik; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues:
⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: 3e385c0d6ce88ac9916dcf84267bd5855d830748
Note: The patch differs from the upstream commit:
---
1: 3e385c0d6ce88 ! 1: a6b033d98441d virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
@@ Metadata
## Commit message ##
virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
- Compared to the SNP Guest Request, the "Extended" version adds data pages for
- receiving certificates. If not enough pages provided, the HV can report to the
- VM how much is needed so the VM can reallocate and repeat.
+ Compared to the SNP Guest Request, the "Extended" version adds data pages
+ for receiving certificates. If not enough pages provided, the HV can
+ report to the VM how much is needed so the VM can reallocate and repeat.
- Commit
-
- ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
-
- moved handling of the allocated/desired pages number out of scope of said
- mutex and create a possibility for a race (multiple instances trying to
- trigger Extended request in a VM) as there is just one instance of
- snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
+ Commit ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command
+ mutex") moved handling of the allocated/desired pages number out of scope
+ of said mutex and create a possibility for a race (multiple instances
+ trying to trigger Extended request in a VM) as there is just one instance
+ of snp_msg_desc per /dev/sev-guest and no locking other than snp_cmd_mutex.
Fix the issue by moving the data blob/size and the GHCB input struct
- (snp_req_data) into snp_guest_req which is allocated on stack now and accessed
- by the GHCB caller under that mutex.
+ (snp_req_data) into snp_guest_req which is allocated on stack now
+ and accessed by the GHCB caller under that mutex.
- Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of four
- callers needs it. Free the received blob in get_ext_report() right after it is
- copied to the userspace. Possible future users of snp_send_guest_request() are
- likely to have different ideas about the buffer size anyways.
+ Stop allocating SEV_FW_BLOB_MAX_SIZE in snp_msg_alloc() as only one of
+ four callers needs it. Free the received blob in get_ext_report() right
+ after it is copied to the userspace. Possible future users of
+ snp_send_guest_request() are likely to have different ideas about
+ the buffer size anyways.
Fixes: ae596615d93d ("virt: sev-guest: Reduce the scope of SNP command mutex")
+ Cc: stable@vger.kernel.org # 6.13
+ Cc: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
- Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
- Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
- Cc: stable@vger.kernel.org
- Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@amd.com
- ## arch/x86/coco/sev/core.c ##
-@@ arch/x86/coco/sev/core.c: struct snp_msg_desc *snp_msg_alloc(void)
- if (!mdesc->response)
- goto e_free_request;
+ ## arch/x86/include/asm/sev.h ##
+@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
+ unsigned int vmpck_id;
+ u8 msg_version;
+ u8 msg_type;
++
++ struct snp_req_data input;
++ void *certs_data;
+ };
-- mdesc->certs_data = alloc_shared_pages(SEV_FW_BLOB_MAX_SIZE);
-- if (!mdesc->certs_data)
-- goto e_free_response;
--
-- /* initial the input address for guest request */
-- mdesc->input.req_gpa = __pa(mdesc->request);
-- mdesc->input.resp_gpa = __pa(mdesc->response);
-- mdesc->input.data_gpa = __pa(mdesc->certs_data);
+ /*
+@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
+ struct snp_guest_msg secret_request, secret_response;
+
+ struct snp_secrets_page *secrets;
+- struct snp_req_data input;
-
- return mdesc;
+- void *certs_data;
--e_free_response:
-- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
- e_free_request:
- free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
- e_unmap:
-@@ arch/x86/coco/sev/core.c: void snp_msg_free(struct snp_msg_desc *mdesc)
- kfree(mdesc->ctx);
- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
- free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
-- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
- iounmap((__force void __iomem *)mdesc->secrets);
+ struct aesgcm_ctx *ctx;
- memset(mdesc, 0, sizeof(*mdesc));
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+
+ ## drivers/virt/coco/sev-guest/sev-guest.c ##
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
switch (rc) {
case -ENOSPC:
/*
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* order to increment the sequence number and thus avoid
* IV reuse.
*/
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
/*
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
}
if (override_npages)
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
return rc;
}
-@@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
- */
- memcpy(mdesc->request, &mdesc->secret_request, sizeof(mdesc->secret_request));
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+ memcpy(mdesc->request, &mdesc->secret_request,
+ sizeof(mdesc->secret_request));
-+ /* Initialize the input address for guest request */
++ /* initial the input address for guest request */
+ req->input.req_gpa = __pa(mdesc->request);
+ req->input.resp_gpa = __pa(mdesc->response);
+ req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
@@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc,
rc = __handle_guest_request(mdesc, req, rio);
if (rc) {
if (rc == -EIO &&
-
- ## arch/x86/include/asm/sev.h ##
-@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
- unsigned int vmpck_id;
- u8 msg_version;
- u8 msg_type;
-+
-+ struct snp_req_data input;
-+ void *certs_data;
- };
-
- /*
-@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
- struct snp_guest_msg secret_request, secret_response;
-
- struct snp_secrets_page *secrets;
-- struct snp_req_data input;
--
-- void *certs_data;
-
- struct aesgcm_ctx *ctx;
-
-
- ## drivers/virt/coco/sev-guest/sev-guest.c ##
@@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
struct snp_guest_req req = {};
int ret, npages = 0, resp_len;
@@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_gu
return ret;
}
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
+ if (!mdesc->response)
+ goto e_free_request;
+
+- mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
+- if (!mdesc->certs_data)
+- goto e_free_response;
+-
+ ret = -EIO;
+ mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
+ if (!mdesc->ctx)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ misc = &snp_dev->misc;
+ misc->minor = MISC_DYNAMIC_MINOR;
+ misc->name = DEVICE_NAME;
+ misc->fops = &snp_guest_fops;
+
+- /* Initialize the input addresses for guest request */
+- mdesc->input.req_gpa = __pa(mdesc->request);
+- mdesc->input.resp_gpa = __pa(mdesc->response);
+- mdesc->input.data_gpa = __pa(mdesc->certs_data);
+-
+ /* Set the privlevel_floor attribute based on the vmpck_id */
+ sev_tsm_ops.privlevel_floor = vmpck_id;
+
+ ret = tsm_register(&sev_tsm_ops, snp_dev);
+ if (ret)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
+ if (ret)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ ret = misc_register(misc);
+ if (ret)
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
+
+ e_free_ctx:
+ kfree(mdesc->ctx);
+-e_free_cert_data:
+- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
+ e_free_response:
+ free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
+ e_free_request:
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static void __exit sev_guest_remove(struct platform_device *pdev)
+ struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
+ struct snp_msg_desc *mdesc = snp_dev->msg_desc;
+
+- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
+ free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
+ free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
+ kfree(mdesc->ctx);
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.13.y | Success | Success |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-11 7:36 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
@ 2025-03-13 9:08 ` Sasha Levin
2025-03-13 11:09 ` Alexey Kardashevskiy
0 siblings, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2025-03-13 9:08 UTC (permalink / raw)
To: stable, aik; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues:
⚠️ Found matching upstream commit but patch is missing proper reference to it
Found matching upstream commit: 3e385c0d6ce88ac9916dcf84267bd5855d830748
Note: The patch differs from the upstream commit:
---
1: 3e385c0d6ce88 ! 1: 796b69e2edd7a virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
@@ Commit message
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
- Cc: stable@vger.kernel.org
+ Cc: stable@vger.kernel.org # 6.13
Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@amd.com
+ (cherry picked from commit 3e385c0d6ce88ac9916dcf84267bd5855d830748)
- ## arch/x86/coco/sev/core.c ##
-@@ arch/x86/coco/sev/core.c: struct snp_msg_desc *snp_msg_alloc(void)
- if (!mdesc->response)
- goto e_free_request;
+ ## arch/x86/include/asm/sev.h ##
+@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
+ unsigned int vmpck_id;
+ u8 msg_version;
+ u8 msg_type;
++
++ struct snp_req_data input;
++ void *certs_data;
+ };
-- mdesc->certs_data = alloc_shared_pages(SEV_FW_BLOB_MAX_SIZE);
-- if (!mdesc->certs_data)
-- goto e_free_response;
--
-- /* initial the input address for guest request */
-- mdesc->input.req_gpa = __pa(mdesc->request);
-- mdesc->input.resp_gpa = __pa(mdesc->response);
-- mdesc->input.data_gpa = __pa(mdesc->certs_data);
+ /*
+@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
+ struct snp_guest_msg secret_request, secret_response;
+
+ struct snp_secrets_page *secrets;
+- struct snp_req_data input;
-
- return mdesc;
+- void *certs_data;
--e_free_response:
-- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
- e_free_request:
- free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
- e_unmap:
-@@ arch/x86/coco/sev/core.c: void snp_msg_free(struct snp_msg_desc *mdesc)
- kfree(mdesc->ctx);
- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
- free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
-- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
- iounmap((__force void __iomem *)mdesc->secrets);
+ struct aesgcm_ctx *ctx;
- memset(mdesc, 0, sizeof(*mdesc));
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+
+ ## drivers/virt/coco/sev-guest/sev-guest.c ##
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* sequence number must be incremented or the VMPCK must be deleted to
* prevent reuse of the IV.
*/
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
switch (rc) {
case -ENOSPC:
/*
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
* order to increment the sequence number and thus avoid
* IV reuse.
*/
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
/*
-@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
}
if (override_npages)
@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
return rc;
}
-@@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
- */
- memcpy(mdesc->request, &mdesc->secret_request, sizeof(mdesc->secret_request));
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
+ memcpy(mdesc->request, &mdesc->secret_request,
+ sizeof(mdesc->secret_request));
-+ /* Initialize the input address for guest request */
++ /* initial the input address for guest request */
+ req->input.req_gpa = __pa(mdesc->request);
+ req->input.resp_gpa = __pa(mdesc->response);
+ req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
@@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc,
rc = __handle_guest_request(mdesc, req, rio);
if (rc) {
if (rc == -EIO &&
-
- ## arch/x86/include/asm/sev.h ##
-@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
- unsigned int vmpck_id;
- u8 msg_version;
- u8 msg_type;
-+
-+ struct snp_req_data input;
-+ void *certs_data;
- };
-
- /*
-@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
- struct snp_guest_msg secret_request, secret_response;
-
- struct snp_secrets_page *secrets;
-- struct snp_req_data input;
--
-- void *certs_data;
-
- struct aesgcm_ctx *ctx;
-
-
- ## drivers/virt/coco/sev-guest/sev-guest.c ##
@@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
struct snp_guest_req req = {};
int ret, npages = 0, resp_len;
@@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_gu
return ret;
}
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
+ if (!mdesc->response)
+ goto e_free_request;
+
+- mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
+- if (!mdesc->certs_data)
+- goto e_free_response;
+-
+ ret = -EIO;
+ mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
+ if (!mdesc->ctx)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ misc = &snp_dev->misc;
+ misc->minor = MISC_DYNAMIC_MINOR;
+ misc->name = DEVICE_NAME;
+ misc->fops = &snp_guest_fops;
+
+- /* Initialize the input addresses for guest request */
+- mdesc->input.req_gpa = __pa(mdesc->request);
+- mdesc->input.resp_gpa = __pa(mdesc->response);
+- mdesc->input.data_gpa = __pa(mdesc->certs_data);
+-
+ /* Set the privlevel_floor attribute based on the vmpck_id */
+ sev_tsm_ops.privlevel_floor = vmpck_id;
+
+ ret = tsm_register(&sev_tsm_ops, snp_dev);
+ if (ret)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
+ if (ret)
+- goto e_free_cert_data;
++ goto e_free_response;
+
+ ret = misc_register(misc);
+ if (ret)
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
+
+ e_free_ctx:
+ kfree(mdesc->ctx);
+-e_free_cert_data:
+- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
+ e_free_response:
+ free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
+ e_free_request:
+@@ drivers/virt/coco/sev-guest/sev-guest.c: static void __exit sev_guest_remove(struct platform_device *pdev)
+ struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
+ struct snp_msg_desc *mdesc = snp_dev->msg_desc;
+
+- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
+ free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
+ free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
+ kfree(mdesc->ctx);
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.13.y | Success | Success |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-13 9:08 ` Sasha Levin
@ 2025-03-13 11:09 ` Alexey Kardashevskiy
2025-03-13 11:34 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Kardashevskiy @ 2025-03-13 11:09 UTC (permalink / raw)
To: Sasha Levin, stable
What does the tool not like in particular? The fact they are different?
Or the backported commit log must have started with "commit xxx
upstream." and "(cherry picked from commit xxx)" is not good enough? Thanks,
On 13/3/25 20:08, Sasha Levin wrote:
> [ Sasha's backport helper bot ]
>
> Hi,
>
> Summary of potential issues:
> ⚠️ Found matching upstream commit but patch is missing proper reference to it
>
> Found matching upstream commit: 3e385c0d6ce88ac9916dcf84267bd5855d830748
>
> Note: The patch differs from the upstream commit:
> ---
> 1: 3e385c0d6ce88 ! 1: 796b69e2edd7a virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
> @@ Commit message
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> - Cc: stable@vger.kernel.org
> + Cc: stable@vger.kernel.org # 6.13
> Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@amd.com
> + (cherry picked from commit 3e385c0d6ce88ac9916dcf84267bd5855d830748)
>
> - ## arch/x86/coco/sev/core.c ##
> -@@ arch/x86/coco/sev/core.c: struct snp_msg_desc *snp_msg_alloc(void)
> - if (!mdesc->response)
> - goto e_free_request;
> + ## arch/x86/include/asm/sev.h ##
> +@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
> + unsigned int vmpck_id;
> + u8 msg_version;
> + u8 msg_type;
> ++
> ++ struct snp_req_data input;
> ++ void *certs_data;
> + };
>
> -- mdesc->certs_data = alloc_shared_pages(SEV_FW_BLOB_MAX_SIZE);
> -- if (!mdesc->certs_data)
> -- goto e_free_response;
> --
> -- /* initial the input address for guest request */
> -- mdesc->input.req_gpa = __pa(mdesc->request);
> -- mdesc->input.resp_gpa = __pa(mdesc->response);
> -- mdesc->input.data_gpa = __pa(mdesc->certs_data);
> + /*
> +@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
> + struct snp_guest_msg secret_request, secret_response;
> +
> + struct snp_secrets_page *secrets;
> +- struct snp_req_data input;
> -
> - return mdesc;
> +- void *certs_data;
>
> --e_free_response:
> -- free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> - e_free_request:
> - free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
> - e_unmap:
> -@@ arch/x86/coco/sev/core.c: void snp_msg_free(struct snp_msg_desc *mdesc)
> - kfree(mdesc->ctx);
> - free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> - free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
> -- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
> - iounmap((__force void __iomem *)mdesc->secrets);
> + struct aesgcm_ctx *ctx;
>
> - memset(mdesc, 0, sizeof(*mdesc));
> -@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> +
> + ## drivers/virt/coco/sev-guest/sev-guest.c ##
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> * sequence number must be incremented or the VMPCK must be deleted to
> * prevent reuse of the IV.
> */
> @@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
> switch (rc) {
> case -ENOSPC:
> /*
> -@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> * order to increment the sequence number and thus avoid
> * IV reuse.
> */
> @@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
> req->exit_code = SVM_VMGEXIT_GUEST_REQUEST;
>
> /*
> -@@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __handle_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> }
>
> if (override_npages)
> @@ arch/x86/coco/sev/core.c: static int __handle_guest_request(struct snp_msg_desc
>
> return rc;
> }
> -@@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
> - */
> - memcpy(mdesc->request, &mdesc->secret_request, sizeof(mdesc->secret_request));
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_r
> + memcpy(mdesc->request, &mdesc->secret_request,
> + sizeof(mdesc->secret_request));
>
> -+ /* Initialize the input address for guest request */
> ++ /* initial the input address for guest request */
> + req->input.req_gpa = __pa(mdesc->request);
> + req->input.resp_gpa = __pa(mdesc->response);
> + req->input.data_gpa = req->certs_data ? __pa(req->certs_data) : 0;
> @@ arch/x86/coco/sev/core.c: int snp_send_guest_request(struct snp_msg_desc *mdesc,
> rc = __handle_guest_request(mdesc, req, rio);
> if (rc) {
> if (rc == -EIO &&
> -
> - ## arch/x86/include/asm/sev.h ##
> -@@ arch/x86/include/asm/sev.h: struct snp_guest_req {
> - unsigned int vmpck_id;
> - u8 msg_version;
> - u8 msg_type;
> -+
> -+ struct snp_req_data input;
> -+ void *certs_data;
> - };
> -
> - /*
> -@@ arch/x86/include/asm/sev.h: struct snp_msg_desc {
> - struct snp_guest_msg secret_request, secret_response;
> -
> - struct snp_secrets_page *secrets;
> -- struct snp_req_data input;
> --
> -- void *certs_data;
> -
> - struct aesgcm_ctx *ctx;
> -
> -
> - ## drivers/virt/coco/sev-guest/sev-guest.c ##
> @@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> struct snp_guest_req req = {};
> int ret, npages = 0, resp_len;
> @@ drivers/virt/coco/sev-guest/sev-guest.c: static int get_ext_report(struct snp_gu
> return ret;
> }
>
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
> + if (!mdesc->response)
> + goto e_free_request;
> +
> +- mdesc->certs_data = alloc_shared_pages(dev, SEV_FW_BLOB_MAX_SIZE);
> +- if (!mdesc->certs_data)
> +- goto e_free_response;
> +-
> + ret = -EIO;
> + mdesc->ctx = snp_init_crypto(mdesc->vmpck, VMPCK_KEY_LEN);
> + if (!mdesc->ctx)
> +- goto e_free_cert_data;
> ++ goto e_free_response;
> +
> + misc = &snp_dev->misc;
> + misc->minor = MISC_DYNAMIC_MINOR;
> + misc->name = DEVICE_NAME;
> + misc->fops = &snp_guest_fops;
> +
> +- /* Initialize the input addresses for guest request */
> +- mdesc->input.req_gpa = __pa(mdesc->request);
> +- mdesc->input.resp_gpa = __pa(mdesc->response);
> +- mdesc->input.data_gpa = __pa(mdesc->certs_data);
> +-
> + /* Set the privlevel_floor attribute based on the vmpck_id */
> + sev_tsm_ops.privlevel_floor = vmpck_id;
> +
> + ret = tsm_register(&sev_tsm_ops, snp_dev);
> + if (ret)
> +- goto e_free_cert_data;
> ++ goto e_free_response;
> +
> + ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
> + if (ret)
> +- goto e_free_cert_data;
> ++ goto e_free_response;
> +
> + ret = misc_register(misc);
> + if (ret)
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static int __init sev_guest_probe(struct platform_device *pdev)
> +
> + e_free_ctx:
> + kfree(mdesc->ctx);
> +-e_free_cert_data:
> +- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
> + e_free_response:
> + free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> + e_free_request:
> +@@ drivers/virt/coco/sev-guest/sev-guest.c: static void __exit sev_guest_remove(struct platform_device *pdev)
> + struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
> + struct snp_msg_desc *mdesc = snp_dev->msg_desc;
> +
> +- free_shared_pages(mdesc->certs_data, SEV_FW_BLOB_MAX_SIZE);
> + free_shared_pages(mdesc->response, sizeof(struct snp_guest_msg));
> + free_shared_pages(mdesc->request, sizeof(struct snp_guest_msg));
> + kfree(mdesc->ctx);
> ---
>
> Results of testing on various branches:
>
> | Branch | Patch Apply | Build Test |
> |---------------------------|-------------|------------|
> | stable/linux-6.13.y | Success | Success |
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-13 11:09 ` Alexey Kardashevskiy
@ 2025-03-13 11:34 ` Greg KH
2025-03-13 12:32 ` Alexey Kardashevskiy
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2025-03-13 11:34 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Sasha Levin, stable
On Thu, Mar 13, 2025 at 10:09:25PM +1100, Alexey Kardashevskiy wrote:
> What does the tool not like in particular? The fact they are different? Or
> the backported commit log must have started with "commit xxx upstream." and
> "(cherry picked from commit xxx)" is not good enough? Thanks,
This is showing that the diff is totally different from what is
upstream. Why is it so different? Why did you not list your changes?
Are you sure you got the git commit id correct?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
2025-03-13 11:34 ` Greg KH
@ 2025-03-13 12:32 ` Alexey Kardashevskiy
0 siblings, 0 replies; 11+ messages in thread
From: Alexey Kardashevskiy @ 2025-03-13 12:32 UTC (permalink / raw)
To: Greg KH; +Cc: Sasha Levin, stable
On 13/3/25 22:34, Greg KH wrote:
> On Thu, Mar 13, 2025 at 10:09:25PM +1100, Alexey Kardashevskiy wrote:
>> What does the tool not like in particular? The fact they are different? Or
>> the backported commit log must have started with "commit xxx upstream." and
>> "(cherry picked from commit xxx)" is not good enough? Thanks,
>
> This is showing that the diff is totally different from what is
> upstream. Why is it so different? Why did you not list your changes?
A bunch of code moved from one file (sev-guest.c) to another (core.c)
after v6.13, but otherwise the two patches are identical, I checked
chunk by chunk. But yeah, when compared via diff, it is not that
obvious, I agree.
> Are you sure you got the git commit id correct?
Yes.
>
> thanks,
>
> greg k-h
--
Alexey
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-13 12:32 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-09 10:29 FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree gregkh
2025-03-10 10:00 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2025-03-10 10:13 ` Alexey Kardashevskiy
2025-03-10 10:35 ` Greg KH
2025-03-13 9:01 ` Sasha Levin
2025-03-10 15:35 ` FAILED: patch "[PATCH] virt: sev-guest: Move SNP Guest Request data pages handling" failed to apply to 6.13-stable tree Borislav Petkov
2025-03-11 7:36 ` [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex Alexey Kardashevskiy
2025-03-13 9:08 ` Sasha Levin
2025-03-13 11:09 ` Alexey Kardashevskiy
2025-03-13 11:34 ` Greg KH
2025-03-13 12:32 ` Alexey Kardashevskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox