[ 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@xxxxxxxxxxxxxxx # 6.13 + Cc: Nikunj A Dadhania <nikunj@xxxxxxx> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx> - Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx> - Reviewed-by: Nikunj A Dadhania <nikunj@xxxxxxx> - Cc: stable@xxxxxxxxxxxxxxx - Link: https://lore.kernel.org/r/20250307013700.437505-3-aik@xxxxxxx - ## 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 |