char: tmp: fix potential null pointer dereference

PATCH In review Gustavo A. R. Silva 2 participants 7 messages May 30, 2017 – Jun 19, 2017

Subsystems (2)

Arm/Microchip (AT91) SoC support TPM device driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-05-30 21:51 UTC #0 lore raw

PATCH 1/1 char: tmp: fix potential null pointer dereference

NULL check at line 147: if (chip) {, implies chip might be NULL. Function dev_get_drvdata() dereference pointer chip. Move pointer priv assignment inside the IF block that checks pointer chip.

Addresses-Coverity-ID: 1397646

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/char/tpm/tpm_atmel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm_atmel.c +2 -1

index 0d322ab..0826efd 100644
@@ -142,9 +142,10 @@ static struct platform_device *pdev;
 static void atml_plat_remove(void)
 {
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
-	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
+	struct tpm_atmel_priv *priv;
 
 	if (chip) {
+		priv = dev_get_drvdata(&chip->dev);
 		tpm_chip_unregister(chip);
 		if (priv->have_region)
 			atmel_release_region(priv->base, priv->region_size);

Signature · 1 line

2.5.0

Jarkko Sakkinen

2017-05-31 12:11 UTC #1 lore raw

Quoted 7 lines · Gustavo A. R. Silva

NULL check at line 147: if (chip) {, implies chip might be NULL. Function dev_get_drvdata() dereference pointer chip. Move pointer priv assignment inside the IF block that checks pointer chip.
Addresses-Coverity-ID: 1397646
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>

It cannot be.

/Jarkko

Quoted 23 lines

---
 drivers/char/tpm/tpm_atmel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm_atmel.c +2 -1

index 0d322ab..0826efd 100644
@@ -142,9 +142,10 @@ static struct platform_device *pdev;
 static void atml_plat_remove(void)
 {
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
-	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
+	struct tpm_atmel_priv *priv;
 
 	if (chip) {
+		priv = dev_get_drvdata(&chip->dev);
 		tpm_chip_unregister(chip);
 		if (priv->have_region)
 			atmel_release_region(priv->base, priv->region_size);

Signature · 1 line

2.5.0

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Gustavo A. R. Silva

author

2017-06-12 22:25 UTC #2 lore raw

Hi Jarkko,

Please, see my comments below

Quoting Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>:

Quoted 11 lines · Gustavo A. R. Silva

Quoted 7 lines
NULL check at line 147: if (chip) {, implies chip might be NULL. Function dev_get_drvdata() dereference pointer chip. Move pointer priv assignment inside the IF block that checks pointer chip.
Addresses-Coverity-ID: 1397646
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>
It cannot be.

I got it.

Quoted 17 lines

/Jarkko

Quoted 15 lines

---
 drivers/char/tpm/tpm_atmel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm_atmel.c +1 -1

index 0d322ab..0826efd 100644
@@ -142,9 +142,10 @@ static struct platform_device *pdev;
 static void atml_plat_remove(void)
 {
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
-	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
+	struct tpm_atmel_priv *priv;

Quoted 1 line

drivers/char/tpm/tpm_atmel.c static struct platform_device *pdev; · line 147
Quoted 1 line
drivers/char/tpm/tpm_atmel.c +0 -0
 	if (chip) {

So, this NULL check could be removed?

Quoted 7 lines

drivers/char/tpm/tpm_atmel.c +1 -1

+		priv = dev_get_drvdata(&chip->dev);
 		tpm_chip_unregister(chip);
 		if (priv->have_region)
 			atmel_release_region(priv->base, priv->region_size);
--

2.5.0

Thank you
--
Gustavo A. R. Silva

Jarkko Sakkinen

2017-06-13 18:03 UTC #3 lore raw

Quoted 37 lines · Gustavo A. R. Silva

Hi Jarkko,
Please, see my comments below
Quoting Jarkko Sakkinen <[email protected]>:
Quoted 11 lines · Gustavo A. R. Silva
Quoted 7 lines
NULL check at line 147: if (chip) {, implies chip might be NULL. Function dev_get_drvdata() dereference pointer chip. Move pointer priv assignment inside the IF block that checks pointer chip.
Addresses-Coverity-ID: 1397646
Signed-off-by Gustavo A. R. Silva <[email protected]>
It cannot be.
I got it.
Quoted 17 lines
/Jarkko
Quoted 15 lines
---
 drivers/char/tpm/tpm_atmel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/char/tpm/tpm_atmel.c +1 -1
index 0d322ab..0826efd 100644
@@ -142,9 +142,10 @@ static struct platform_device *pdev;
 static void atml_plat_remove(void)
 {
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
-	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
+	struct tpm_atmel_priv *priv;
 

Quoted 3 lines

Quoted 1 line
drivers/char/tpm/tpm_atmel.c static struct platform_device *pdev; · line 147
Quoted 1 line
drivers/char/tpm/tpm_atmel.c +0 -0
 	if (chip) {
So, this NULL check could be removed?

Yes, this would be right way to fix it.

/Jarkko

Gustavo A. R. Silva

author

2017-06-13 19:55 UTC #4 lore raw

PATCH 1/1 char: tpm: remove unnecessary NULL check

Remove unnecessary NULL check.
Pointer _chip_ cannot be NULL in this instance.

Addresses-Coverity-ID: 1397646

Cc Jarkko Sakkinen <[email protected]>

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/char/tpm/tpm_atmel.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/char/tpm/tpm_atmel.c +5 -7

index 0d322ab..66a1452 100644
@@ -144,13 +144,11 @@ static void atml_plat_remove(void)
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
 	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
 
-	if (chip) {
-		tpm_chip_unregister(chip);
-		if (priv->have_region)
-			atmel_release_region(priv->base, priv->region_size);
-		atmel_put_base_addr(priv->iobase);
-		platform_device_unregister(pdev);
-	}
+	tpm_chip_unregister(chip);
+	if (priv->have_region)
+		atmel_release_region(priv->base, priv->region_size);
+	atmel_put_base_addr(priv->iobase);
+	platform_device_unregister(pdev);
 }
 
 static SIMPLE_DEV_PM_OPS(tpm_atml_pm, tpm_pm_suspend, tpm_pm_resume);

Signature · 1 line

2.5.0

Jarkko Sakkinen

Reviewed-by on v1 1/1

2017-06-14 09:46 UTC #5 lore raw

Re: [PATCH] char: tpm: remove unnecessary NULL check

Quoted 36 lines · Gustavo A. R. Silva

Remove unnecessary NULL check.
Pointer _chip_ cannot be NULL in this instance.

Addresses-Coverity-ID: 1397646

Cc Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>

Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>

---
 drivers/char/tpm/tpm_atmel.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/char/tpm/tpm_atmel.c +5 -7

index 0d322ab..66a1452 100644
@@ -144,13 +144,11 @@ static void atml_plat_remove(void)
 	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
 	struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
 
-	if (chip) {
-		tpm_chip_unregister(chip);
-		if (priv->have_region)
-			atmel_release_region(priv->base, priv->region_size);
-		atmel_put_base_addr(priv->iobase);
-		platform_device_unregister(pdev);
-	}
+	tpm_chip_unregister(chip);
+	if (priv->have_region)
+		atmel_release_region(priv->base, priv->region_size);
+	atmel_put_base_addr(priv->iobase);
+	platform_device_unregister(pdev);
 }
 
 static SIMPLE_DEV_PM_OPS(tpm_atml_pm, tpm_pm_suspend, tpm_pm_resume);

Signature · 1 line

2.5.0

Thank you.

Reviewed-by Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>

/Jarkko

Jarkko Sakkinen

Reviewed-by Tested-by on v1 1/1

2017-06-19 00:35 UTC #6 lore raw

Re: [PATCH] char: tpm: remove unnecessary NULL check

In reply to Gustavo A. R. Silva

Quoted 6 lines · Gustavo A. R. Silva

Remove unnecessary NULL check.
Pointer _chip_ cannot be NULL in this instance.

Addresses-Coverity-ID: 1397646
Cc Jarkko Sakkinen <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

Tested-by Jarkko Sakkinen <[email protected]>

(compilation)

/Jarkko