Re: [PATCH 6.13.y] virt: sev-guest: Move SNP Guest Request data pages handling under snp_cmd_mutex

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux