All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: stable@vger.kernel.org, aik@amd.com
Cc: Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex
Date: Thu, 13 Mar 2025 05:01:44 -0400	[thread overview]
Message-ID: <20250312224217-281d7f5ef03b0f1c@stable.kernel.org> (raw)
In-Reply-To: <20250310100027.1228858-1-aik@amd.com>

[ 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   |

  parent reply	other threads:[~2025-03-13  9:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250312224217-281d7f5ef03b0f1c@stable.kernel.org \
    --to=sashal@kernel.org \
    --cc=aik@amd.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.