All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit
@ 2020-03-03 13:57 John Allen
  2020-03-03 13:57 ` [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit() John Allen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: John Allen @ 2020-03-03 13:57 UTC (permalink / raw)
  To: linux-crypto
  Cc: thomas.lendacky, herbert, davem, brijesh.singh, bp, linux-kernel,
	John Allen

Stale pointers left in static variables misc_dev and sp_dev_master will
trigger use-after-free error when DEBUG_TEST_DRIVER_REMOVE is configured.
                                                                                 
John Allen (2):
  crypto/ccp: Cleanup misc_dev on sev exit
  crypto/ccp: Cleanup sp_dev_master on psp device destroy

 drivers/crypto/ccp/psp-dev.c | 3 +++
 drivers/crypto/ccp/sev-dev.c | 6 +++---
 drivers/crypto/ccp/sp-dev.h  | 1 +
 drivers/crypto/ccp/sp-pci.c  | 9 +++++++++
 4 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.18.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit()
  2020-03-03 13:57 [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit John Allen
@ 2020-03-03 13:57 ` John Allen
  2020-03-06 15:56   ` Tom Lendacky
  2020-03-03 13:57 ` [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy() John Allen
  2020-03-12 12:38 ` [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit Herbert Xu
  2 siblings, 1 reply; 6+ messages in thread
From: John Allen @ 2020-03-03 13:57 UTC (permalink / raw)
  To: linux-crypto
  Cc: thomas.lendacky, herbert, davem, brijesh.singh, bp, linux-kernel,
	John Allen

Explicitly free and clear misc_dev in sev_exit(). Since devm_kzalloc()
associates misc_dev with the first device that gets probed, change from
devm_kzalloc() to kzalloc() and explicitly free memory in sev_exit() as
the first device probed is not guaranteed to be the last device released.
To ensure that the variable gets properly set to NULL, remove the local
definition of misc_dev.

Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
Signed-off-by: John Allen <john.allen@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e467860f797d..aa591dae067c 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -896,9 +896,9 @@ EXPORT_SYMBOL_GPL(sev_guest_df_flush);
 
 static void sev_exit(struct kref *ref)
 {
-	struct sev_misc_dev *misc_dev = container_of(ref, struct sev_misc_dev, refcount);
-
 	misc_deregister(&misc_dev->misc);
+	kfree(misc_dev);
+	misc_dev = NULL;
 }
 
 static int sev_misc_init(struct sev_device *sev)
@@ -916,7 +916,7 @@ static int sev_misc_init(struct sev_device *sev)
 	if (!misc_dev) {
 		struct miscdevice *misc;
 
-		misc_dev = devm_kzalloc(dev, sizeof(*misc_dev), GFP_KERNEL);
+		misc_dev = kzalloc(sizeof(*misc_dev), GFP_KERNEL);
 		if (!misc_dev)
 			return -ENOMEM;
 
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy()
  2020-03-03 13:57 [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit John Allen
  2020-03-03 13:57 ` [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit() John Allen
@ 2020-03-03 13:57 ` John Allen
  2020-03-06 15:57   ` Tom Lendacky
  2020-03-12 12:38 ` [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit Herbert Xu
  2 siblings, 1 reply; 6+ messages in thread
From: John Allen @ 2020-03-03 13:57 UTC (permalink / raw)
  To: linux-crypto
  Cc: thomas.lendacky, herbert, davem, brijesh.singh, bp, linux-kernel,
	John Allen

Introduce clear_psp_master_device() to ensure that sp_dev_master gets
properly cleared on the release of a psp device.

Fixes: 2a6170dfe755 ("crypto: ccp: Add Platform Security Processor (PSP) device support")
Signed-off-by: John Allen <john.allen@amd.com>
---
 drivers/crypto/ccp/psp-dev.c | 3 +++
 drivers/crypto/ccp/sp-dev.h  | 1 +
 drivers/crypto/ccp/sp-pci.c  | 9 +++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index e95e7aa5dbf1..ae7b44599914 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -215,6 +215,9 @@ void psp_dev_destroy(struct sp_device *sp)
 	tee_dev_destroy(psp);
 
 	sp_free_psp_irq(sp, psp);
+
+	if (sp->clear_psp_master_device)
+		sp->clear_psp_master_device(sp);
 }
 
 void psp_set_sev_irq_handler(struct psp_device *psp, psp_irq_handler_t handler,
diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
index 423594608ad1..f913f1494af9 100644
--- a/drivers/crypto/ccp/sp-dev.h
+++ b/drivers/crypto/ccp/sp-dev.h
@@ -90,6 +90,7 @@ struct sp_device {
 	/* get and set master device */
 	struct sp_device*(*get_psp_master_device)(void);
 	void (*set_psp_master_device)(struct sp_device *);
+	void (*clear_psp_master_device)(struct sp_device *);
 
 	bool irq_registered;
 	bool use_tasklet;
diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 56c1f61c0f84..cb6cb47053f4 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -146,6 +146,14 @@ static struct sp_device *psp_get_master(void)
 	return sp_dev_master;
 }
 
+static void psp_clear_master(struct sp_device *sp)
+{
+	if (sp == sp_dev_master) {
+		sp_dev_master = NULL;
+		dev_dbg(sp->dev, "Cleared sp_dev_master\n");
+	}
+}
+
 static int sp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct sp_device *sp;
@@ -206,6 +214,7 @@ static int sp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_set_master(pdev);
 	sp->set_psp_master_device = psp_set_master;
 	sp->get_psp_master_device = psp_get_master;
+	sp->clear_psp_master_device = psp_clear_master;
 
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
 	if (ret) {
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit()
  2020-03-03 13:57 ` [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit() John Allen
@ 2020-03-06 15:56   ` Tom Lendacky
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Lendacky @ 2020-03-06 15:56 UTC (permalink / raw)
  To: John Allen, linux-crypto; +Cc: herbert, davem, brijesh.singh, bp, linux-kernel

On 3/3/20 7:57 AM, John Allen wrote:
> Explicitly free and clear misc_dev in sev_exit(). Since devm_kzalloc()
> associates misc_dev with the first device that gets probed, change from
> devm_kzalloc() to kzalloc() and explicitly free memory in sev_exit() as
> the first device probed is not guaranteed to be the last device released.
> To ensure that the variable gets properly set to NULL, remove the local
> definition of misc_dev.
> 
> Fixes: 200664d5237f ("crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support")
> Signed-off-by: John Allen <john.allen@amd.com>

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

> ---
>  drivers/crypto/ccp/sev-dev.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index e467860f797d..aa591dae067c 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -896,9 +896,9 @@ EXPORT_SYMBOL_GPL(sev_guest_df_flush);
>  
>  static void sev_exit(struct kref *ref)
>  {
> -	struct sev_misc_dev *misc_dev = container_of(ref, struct sev_misc_dev, refcount);
> -
>  	misc_deregister(&misc_dev->misc);
> +	kfree(misc_dev);
> +	misc_dev = NULL;
>  }
>  
>  static int sev_misc_init(struct sev_device *sev)
> @@ -916,7 +916,7 @@ static int sev_misc_init(struct sev_device *sev)
>  	if (!misc_dev) {
>  		struct miscdevice *misc;
>  
> -		misc_dev = devm_kzalloc(dev, sizeof(*misc_dev), GFP_KERNEL);
> +		misc_dev = kzalloc(sizeof(*misc_dev), GFP_KERNEL);
>  		if (!misc_dev)
>  			return -ENOMEM;
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy()
  2020-03-03 13:57 ` [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy() John Allen
@ 2020-03-06 15:57   ` Tom Lendacky
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Lendacky @ 2020-03-06 15:57 UTC (permalink / raw)
  To: John Allen, linux-crypto; +Cc: herbert, davem, brijesh.singh, bp, linux-kernel

On 3/3/20 7:57 AM, John Allen wrote:
> Introduce clear_psp_master_device() to ensure that sp_dev_master gets
> properly cleared on the release of a psp device.
> 
> Fixes: 2a6170dfe755 ("crypto: ccp: Add Platform Security Processor (PSP) device support")
> Signed-off-by: John Allen <john.allen@amd.com>

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

> ---
>  drivers/crypto/ccp/psp-dev.c | 3 +++
>  drivers/crypto/ccp/sp-dev.h  | 1 +
>  drivers/crypto/ccp/sp-pci.c  | 9 +++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index e95e7aa5dbf1..ae7b44599914 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -215,6 +215,9 @@ void psp_dev_destroy(struct sp_device *sp)
>  	tee_dev_destroy(psp);
>  
>  	sp_free_psp_irq(sp, psp);
> +
> +	if (sp->clear_psp_master_device)
> +		sp->clear_psp_master_device(sp);
>  }
>  
>  void psp_set_sev_irq_handler(struct psp_device *psp, psp_irq_handler_t handler,
> diff --git a/drivers/crypto/ccp/sp-dev.h b/drivers/crypto/ccp/sp-dev.h
> index 423594608ad1..f913f1494af9 100644
> --- a/drivers/crypto/ccp/sp-dev.h
> +++ b/drivers/crypto/ccp/sp-dev.h
> @@ -90,6 +90,7 @@ struct sp_device {
>  	/* get and set master device */
>  	struct sp_device*(*get_psp_master_device)(void);
>  	void (*set_psp_master_device)(struct sp_device *);
> +	void (*clear_psp_master_device)(struct sp_device *);
>  
>  	bool irq_registered;
>  	bool use_tasklet;
> diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
> index 56c1f61c0f84..cb6cb47053f4 100644
> --- a/drivers/crypto/ccp/sp-pci.c
> +++ b/drivers/crypto/ccp/sp-pci.c
> @@ -146,6 +146,14 @@ static struct sp_device *psp_get_master(void)
>  	return sp_dev_master;
>  }
>  
> +static void psp_clear_master(struct sp_device *sp)
> +{
> +	if (sp == sp_dev_master) {
> +		sp_dev_master = NULL;
> +		dev_dbg(sp->dev, "Cleared sp_dev_master\n");
> +	}
> +}
> +
>  static int sp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	struct sp_device *sp;
> @@ -206,6 +214,7 @@ static int sp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	pci_set_master(pdev);
>  	sp->set_psp_master_device = psp_set_master;
>  	sp->get_psp_master_device = psp_get_master;
> +	sp->clear_psp_master_device = psp_clear_master;
>  
>  	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
>  	if (ret) {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit
  2020-03-03 13:57 [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit John Allen
  2020-03-03 13:57 ` [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit() John Allen
  2020-03-03 13:57 ` [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy() John Allen
@ 2020-03-12 12:38 ` Herbert Xu
  2 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2020-03-12 12:38 UTC (permalink / raw)
  To: John Allen
  Cc: linux-crypto, thomas.lendacky, davem, brijesh.singh, bp, linux-kernel

On Tue, Mar 03, 2020 at 07:57:22AM -0600, John Allen wrote:
> Stale pointers left in static variables misc_dev and sp_dev_master will
> trigger use-after-free error when DEBUG_TEST_DRIVER_REMOVE is configured.
>                                                                                  
> John Allen (2):
>   crypto/ccp: Cleanup misc_dev on sev exit
>   crypto/ccp: Cleanup sp_dev_master on psp device destroy
> 
>  drivers/crypto/ccp/psp-dev.c | 3 +++
>  drivers/crypto/ccp/sev-dev.c | 6 +++---
>  drivers/crypto/ccp/sp-dev.h  | 1 +
>  drivers/crypto/ccp/sp-pci.c  | 9 +++++++++
>  4 files changed, 16 insertions(+), 3 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-12 12:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03 13:57 [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit John Allen
2020-03-03 13:57 ` [PATCH 1/2] crypto/ccp: Cleanup misc_dev on sev_exit() John Allen
2020-03-06 15:56   ` Tom Lendacky
2020-03-03 13:57 ` [PATCH 2/2] crypto/ccp: Cleanup sp_dev_master in psp_dev_destroy() John Allen
2020-03-06 15:57   ` Tom Lendacky
2020-03-12 12:38 ` [PATCH 0/2] crypto/ccp: Properly NULL variables on sev exit Herbert Xu

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.