[PATCH] tpm_vtpm_proxy: fix race condition in /dev/vtpmx creation

[PATCH] tpm_vtpm_proxy: fix race condition in /dev/vtpmx creation

[Jarkko Sakkinen]

/dev/vtpmx is made visible before the workqueue exist, which can lead
memory corruption in the worst case, if workqueue is used before it has
been fully initialized. Address this by changing the call order.

Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
Fixes: 6f99612e2500 ("tpm: Proxy driver for supporting multiple emulated TPMs")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@tuni.fi>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 5c865987ba5c..ef1367cf2f10 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -50,7 +50,7 @@ struct proxy_dev {
 /* all supported flags */
 #define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
 
-static struct workqueue_struct *workqueue;
+static struct workqueue_struct *vtpm_workqueue;
 
 static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
 
@@ -478,7 +478,7 @@ static void vtpm_proxy_work_stop(struct proxy_dev *proxy_dev)
  */
 static inline void vtpm_proxy_work_start(struct proxy_dev *proxy_dev)
 {
-	queue_work(workqueue, &proxy_dev->work);
+	queue_work(vtpm_workqueue, &proxy_dev->work);
 }
 
 /*
@@ -697,30 +697,24 @@ static int __init vtpm_module_init(void)
 {
 	int rc;
 
-	rc = vtpmx_init();
-	if (rc) {
-		pr_err("couldn't create vtpmx device\n");
+	vtpm_workqueue = create_workqueue("tpm-vtpm");
+	if (!vtpm_workqueue) {
+		rc = -ENOMEM;
 		return rc;
 	}
 
-	workqueue = create_workqueue("tpm-vtpm");
-	if (!workqueue) {
-		pr_err("couldn't create workqueue\n");
-		rc = -ENOMEM;
-		goto err_vtpmx_cleanup;
+	rc = vtpmx_init();
+	if (rc) {
+		vtpmx_cleanup();
+		return rc;
 	}
 
 	return 0;
-
-err_vtpmx_cleanup:
-	vtpmx_cleanup();
-
-	return rc;
 }
 
 static void __exit vtpm_module_exit(void)
 {
-	destroy_workqueue(workqueue);
+	destroy_workqueue(vtpm_workqueue);
 	vtpmx_cleanup();
 }
 
-- 
2.39.2

Re: [PATCH] tpm_vtpm_proxy: fix race condition in /dev/vtpmx creation

[Jarkko Sakkinen]

On Sat May 13, 2023 at 8:28 PM EEST, Jarkko Sakkinen wrote:
> /dev/vtpmx is made visible before the workqueue exist, which can lead
> memory corruption in the worst case, if workqueue is used before it has
> been fully initialized. Address this by changing the call order.
>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: stable@vger.kernel.org
> Fixes: 6f99612e2500 ("tpm: Proxy driver for supporting multiple emulated TPMs")
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@tuni.fi>
> ---
>  drivers/char/tpm/tpm_vtpm_proxy.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 5c865987ba5c..ef1367cf2f10 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -50,7 +50,7 @@ struct proxy_dev {
>  /* all supported flags */
>  #define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
>  
> -static struct workqueue_struct *workqueue;
> +static struct workqueue_struct *vtpm_workqueue;
>  
>  static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev);
>  
> @@ -478,7 +478,7 @@ static void vtpm_proxy_work_stop(struct proxy_dev *proxy_dev)
>   */
>  static inline void vtpm_proxy_work_start(struct proxy_dev *proxy_dev)
>  {
> -	queue_work(workqueue, &proxy_dev->work);
> +	queue_work(vtpm_workqueue, &proxy_dev->work);
>  }
>  
>  /*
> @@ -697,30 +697,24 @@ static int __init vtpm_module_init(void)
>  {
>  	int rc;
>  
> -	rc = vtpmx_init();
> -	if (rc) {
> -		pr_err("couldn't create vtpmx device\n");
> +	vtpm_workqueue = create_workqueue("tpm-vtpm");
> +	if (!vtpm_workqueue) {
> +		rc = -ENOMEM;
>  		return rc;
>  	}
>  
> -	workqueue = create_workqueue("tpm-vtpm");
> -	if (!workqueue) {
> -		pr_err("couldn't create workqueue\n");
> -		rc = -ENOMEM;
> -		goto err_vtpmx_cleanup;
> +	rc = vtpmx_init();
> +	if (rc) {
> +		vtpmx_cleanup();

This should be destroy_workqueue(). I'll send a new version.

BR, Jarkko

Re: [PATCH] tpm_vtpm_proxy: fix race condition in /dev/vtpmx creation

[Stefan Berger]

On 5/13/23 13:28, Jarkko Sakkinen wrote:
> /dev/vtpmx is made visible before the workqueue exist, which can lead

which can lead to

> memory corruption in the worst case, if workqueue is used before it has
> been fully initialized. Address this by changing the call order.
> 

Thanks,
    Stefan