[PATCH] virt: sev-guest: Fix lifetime of message descriptor

Kohei Enju posted 1 patch 1 month ago
drivers/virt/coco/sev-guest/sev-guest.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
[PATCH] virt: sev-guest: Fix lifetime of message descriptor
Posted by Kohei Enju 1 month ago
sev_guest_probe() registers the TSM provider before assigning
snp_dev->msg_desc. Once the provider is registered, a concurrent
configfs report request can reach get_ext_report() and dereference a
NULL message descriptor.

Additionally, sev_guest_remove() frees the message descriptor before the
managed TSM unregister action runs, allowing the provider to access the
freed descriptor.

Assign snp_dev->msg_desc before registering the TSM provider. Add a
managed cleanup action for the message descriptor before adding the TSM
unregister action. Since devres actions run in LIFO order, the TSM
provider is unregistered before the message descriptor is freed both
during probe failure and driver removal.

Fixes: f47906782c76 ("virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT")
Fixes: 0a895c0d9b73 ("virt: sev-guest: Carve out SNP message context structure")
Signed-off-by: Kohei Enju <kohei@enjuk.jp>
---
 drivers/virt/coco/sev-guest/sev-guest.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 935537a41469..458c96fd82f3 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -618,6 +618,11 @@ static struct tsm_report_ops sev_tsm_report_ops = {
 	.report_bin_attr_visible = sev_report_bin_attr_visible,
 };
 
+static void free_snp_msg(void *data)
+{
+	snp_msg_free(data);
+}
+
 static void unregister_sev_tsm(void *data)
 {
 	tsm_report_unregister(&sev_tsm_report_ops);
@@ -644,9 +649,15 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 	if (IS_ERR_OR_NULL(mdesc))
 		return -ENOMEM;
 
+	ret = devm_add_action_or_reset(&pdev->dev, free_snp_msg, mdesc);
+	if (ret)
+		return ret;
+
 	ret = snp_msg_init(mdesc, vmpck_id);
 	if (ret)
-		goto e_msg_init;
+		return ret;
+
+	snp_dev->msg_desc = mdesc;
 
 	platform_set_drvdata(pdev, snp_dev);
 	snp_dev->dev = dev;
@@ -661,32 +672,25 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 
 	ret = tsm_report_register(&sev_tsm_report_ops, snp_dev);
 	if (ret)
-		goto e_msg_init;
+		return ret;
 
 	ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
 	if (ret)
-		goto e_msg_init;
+		return ret;
 
 	ret =  misc_register(misc);
 	if (ret)
-		goto e_msg_init;
+		return ret;
 
-	snp_dev->msg_desc = mdesc;
 	dev_info(dev, "Initialized SEV guest driver (using VMPCK%d communication key)\n",
 		 mdesc->vmpck_id);
 	return 0;
-
-e_msg_init:
-	snp_msg_free(mdesc);
-
-	return ret;
 }
 
 static void __exit sev_guest_remove(struct platform_device *pdev)
 {
 	struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
 
-	snp_msg_free(snp_dev->msg_desc);
 	misc_deregister(&snp_dev->misc);
 }
 

---
base-commit: 26260251022fbc2f248a3d747a9b2b961b18d2d8
change-id: 20260822-sev-guest-mdesc-lifetime-e08772b010d4

Best regards,
-- 
Kohei Enju <kohei@enjuk.jp>
Re: [PATCH] virt: sev-guest: Fix lifetime of message descriptor
Posted by Borislav Petkov 3 weeks, 5 days ago
On Sat, Aug 22, 2026 at 09:35:03PM +0900, Kohei Enju wrote:
> sev_guest_probe() registers the TSM provider before assigning
> snp_dev->msg_desc. Once the provider is registered, a concurrent
> configfs report request can reach get_ext_report() and dereference a
> NULL message descriptor.

How can this happen? This is a driver probe function and someone must very
quickly do that configfs report request?
> 
> Additionally, sev_guest_remove() frees the message descriptor before the
> managed TSM unregister action runs, allowing the provider to access the
> freed descriptor.
> 
> Assign snp_dev->msg_desc before registering the TSM provider. Add a
> managed cleanup action for the message descriptor before adding the TSM
> unregister action. Since devres actions run in LIFO order, the TSM
> provider is unregistered before the message descriptor is freed both
> during probe failure and driver removal.

Can we get rid of those managed actions instead and do proper unwinding?

There's a single devm_add_action_or_reset() right smack in the middle of the
probe flow and I'd prefer if we don't add more but have proper and balanced
unwinding.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] virt: sev-guest: Fix lifetime of message descriptor
Posted by Nikunj A. Dadhania 4 weeks ago

On 8/22/2026 6:05 PM, Kohei Enju wrote:
> sev_guest_probe() registers the TSM provider before assigning
> snp_dev->msg_desc. Once the provider is registered, a concurrent
> configfs report request can reach get_ext_report() and dereference a
> NULL message descriptor.
> 
> Additionally, sev_guest_remove() frees the message descriptor before the
> managed TSM unregister action runs, allowing the provider to access the
> freed descriptor.
> 
> Assign snp_dev->msg_desc before registering the TSM provider. Add a
> managed cleanup action for the message descriptor before adding the TSM
> unregister action. Since devres actions run in LIFO order, the TSM
> provider is unregistered before the message descriptor is freed both
> during probe failure and driver removal.
> 
> Fixes: f47906782c76 ("virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT")
> Fixes: 0a895c0d9b73 ("virt: sev-guest: Carve out SNP message context structure")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

> ---
>  drivers/virt/coco/sev-guest/sev-guest.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 935537a41469..458c96fd82f3 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -618,6 +618,11 @@ static struct tsm_report_ops sev_tsm_report_ops = {
>  	.report_bin_attr_visible = sev_report_bin_attr_visible,
>  };
>  
> +static void free_snp_msg(void *data)
> +{
> +	snp_msg_free(data);
> +}
> +
>  static void unregister_sev_tsm(void *data)
>  {
>  	tsm_report_unregister(&sev_tsm_report_ops);
> @@ -644,9 +649,15 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>  	if (IS_ERR_OR_NULL(mdesc))
>  		return -ENOMEM;
>  
> +	ret = devm_add_action_or_reset(&pdev->dev, free_snp_msg, mdesc);
> +	if (ret)
> +		return ret;
> +
>  	ret = snp_msg_init(mdesc, vmpck_id);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
> +
> +	snp_dev->msg_desc = mdesc;
>  
>  	platform_set_drvdata(pdev, snp_dev);
>  	snp_dev->dev = dev;
> @@ -661,32 +672,25 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>  
>  	ret = tsm_report_register(&sev_tsm_report_ops, snp_dev);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
>  	ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
>  	ret =  misc_register(misc);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
> -	snp_dev->msg_desc = mdesc;
>  	dev_info(dev, "Initialized SEV guest driver (using VMPCK%d communication key)\n",
>  		 mdesc->vmpck_id);
>  	return 0;
> -
> -e_msg_init:
> -	snp_msg_free(mdesc);
> -
> -	return ret;
>  }
>  
>  static void __exit sev_guest_remove(struct platform_device *pdev)
>  {
>  	struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
>  
> -	snp_msg_free(snp_dev->msg_desc);
>  	misc_deregister(&snp_dev->misc);
>  }
>  
> 
> ---
> base-commit: 26260251022fbc2f248a3d747a9b2b961b18d2d8
> change-id: 20260822-sev-guest-mdesc-lifetime-e08772b010d4
> 
> Best regards,
Re: [PATCH] virt: sev-guest: Fix lifetime of message descriptor
Posted by Tom Lendacky 4 weeks, 1 day ago
On 8/22/26 07:35, Kohei Enju wrote:
> sev_guest_probe() registers the TSM provider before assigning
> snp_dev->msg_desc. Once the provider is registered, a concurrent
> configfs report request can reach get_ext_report() and dereference a
> NULL message descriptor.
> 
> Additionally, sev_guest_remove() frees the message descriptor before the
> managed TSM unregister action runs, allowing the provider to access the
> freed descriptor.
> 
> Assign snp_dev->msg_desc before registering the TSM provider. Add a
> managed cleanup action for the message descriptor before adding the TSM
> unregister action. Since devres actions run in LIFO order, the TSM
> provider is unregistered before the message descriptor is freed both
> during probe failure and driver removal.
> 
> Fixes: f47906782c76 ("virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT")
> Fixes: 0a895c0d9b73 ("virt: sev-guest: Carve out SNP message context structure")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/virt/coco/sev-guest/sev-guest.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 935537a41469..458c96fd82f3 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -618,6 +618,11 @@ static struct tsm_report_ops sev_tsm_report_ops = {
>  	.report_bin_attr_visible = sev_report_bin_attr_visible,
>  };
>  
> +static void free_snp_msg(void *data)
> +{
> +	snp_msg_free(data);
> +}
> +
>  static void unregister_sev_tsm(void *data)
>  {
>  	tsm_report_unregister(&sev_tsm_report_ops);
> @@ -644,9 +649,15 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>  	if (IS_ERR_OR_NULL(mdesc))
>  		return -ENOMEM;
>  
> +	ret = devm_add_action_or_reset(&pdev->dev, free_snp_msg, mdesc);
> +	if (ret)
> +		return ret;
> +
>  	ret = snp_msg_init(mdesc, vmpck_id);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
> +
> +	snp_dev->msg_desc = mdesc;
>  
>  	platform_set_drvdata(pdev, snp_dev);
>  	snp_dev->dev = dev;
> @@ -661,32 +672,25 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>  
>  	ret = tsm_report_register(&sev_tsm_report_ops, snp_dev);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
>  	ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
>  	ret =  misc_register(misc);
>  	if (ret)
> -		goto e_msg_init;
> +		return ret;
>  
> -	snp_dev->msg_desc = mdesc;
>  	dev_info(dev, "Initialized SEV guest driver (using VMPCK%d communication key)\n",
>  		 mdesc->vmpck_id);
>  	return 0;
> -
> -e_msg_init:
> -	snp_msg_free(mdesc);
> -
> -	return ret;
>  }
>  
>  static void __exit sev_guest_remove(struct platform_device *pdev)
>  {
>  	struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
>  
> -	snp_msg_free(snp_dev->msg_desc);
>  	misc_deregister(&snp_dev->misc);
>  }
>  
> 
> ---
> base-commit: 26260251022fbc2f248a3d747a9b2b961b18d2d8
> change-id: 20260822-sev-guest-mdesc-lifetime-e08772b010d4
> 
> Best regards,