tpm: Fix lack of driver_unregister in init failcases

PATCH In review Kylene Jo Hall 2 participants 2 messages Oct 31, 2005 – Nov 6, 2005 TPM device driver

Share & tools

Kylene Jo Hall

author

2005-10-31 14:37 UTC #0 lore raw

PATCH 1/1 tpm: Fix lack of driver_unregister in init failcases

driver_unregister is not being properly called when the init function returns an error case. Restructured the return logic such that this and the other cleanups all happen in one place. Preformed many of the cleanups that Andrew Morton's patch on Thursday made in tpm_atmel.c. Fixed Matthieu's concern about writing before discovery.

Signed-off-by Kylene Hall

---

drivers/char/tpm/tpm_nsc.c +25 ~~-26~~

@@ -286,8 +286,4 @@ static int __init init_nsc(void)
 	int lo, hi;
 	int nscAddrBase = TPM_ADDR;
 
-	driver_register(&nsc_drv);
-
- 	/* select PM channel 1 */
-	tpm_write_index(nscAddrBase,NSC_LDN_INDEX, 0x12);

@@ -299,6 +297,8 @@ static int __init init_nsc(void)
 			return -ENODEV;
 	}
 
+	driver_register(&nsc_drv);
+
 	hi = tpm_read_index(nscAddrBase, TPM_NSC_BASE0_HI);
 	lo = tpm_read_index(nscAddrBase, TPM_NSC_BASE0_LO);
 	tpm_nsc.base = (hi<<8) | lo;

@@ -306,38 +306,28 @@ static int __init init_nsc(void)
 	/* enable the DPM module */
 	tpm_write_index(nscAddrBase, NSC_LDC_INDEX, 0x01);
 
-	pdev = kmalloc(sizeof(struct platform_device), GFP_KERNEL);
-	if ( !pdev ) 
-		return -ENOMEM;
-
-	memset(pdev, 0, sizeof(struct platform_device));
+	pdev = kzalloc(sizeof(struct platform_device), GFP_KERNEL);
+	if ( !pdev ) { 
+		rc = -ENOMEM;
+		goto err_unreg_drv;
+	}
 
 	pdev->name = "tpm_nscl0";
 	pdev->id = -1;
 	pdev->num_resources = 0;
-	pdev->dev.release = tpm_nsc_remove;	
+	pdev->dev.release = tpm_nsc_remove;
 	pdev->dev.driver = &nsc_drv;
 
-	if ((rc=platform_device_register(pdev)) < 0) {
-		kfree(pdev);
-		pdev = NULL;
-		return rc;
-	}
+	if ((rc = platform_device_register(pdev)) < 0) 
+		goto err_free_dev;
 
 	if (request_region(tpm_nsc.base, 2, "tpm_nsc0") == NULL ) {
-		platform_device_unregister(pdev);
-		kfree(pdev);
-		pdev = NULL;
-		return -EBUSY;
+		rc = -EBUSY;
+		goto err_unreg_dev;
 	}
 
-	if ((rc = tpm_register_hardware(&pdev->dev, &tpm_nsc)) < 0) {
-		release_region(tpm_nsc.base, 2);
-		platform_device_unregister(pdev);
-		kfree(pdev);
-		pdev = NULL;
-		return rc;
-	}
+	if ((rc = tpm_register_hardware(&pdev->dev, &tpm_nsc)) < 0) 
+		goto err_rel_reg;
 
 	dev_dbg(&pdev->dev, "NSC TPM detected\n");
 	dev_dbg(&pdev->dev,

@@ -373,6 +363,17 @@ static int __init init_nsc(void)
 		 tpm_read_index(nscAddrBase, 0x27) & 0x1F);
 
 	return 0;
+
+err_rel_reg:
+	release_region(tpm_nsc.base, 2);
+err_unreg_dev:
+	platform_device_unregister(pdev);
+err_free_dev:
+	kfree(pdev);
+	pdev = NULL;
+err_unreg_drv:
+	driver_unregister(&nsc_drv);
+	return rc;
 }
 
 static void __exit cleanup_nsc(void)

Russell King

2005-11-06 08:24 UTC #1 lore raw

Quoted 38 lines · Kylene Jo Hall

Signed-off-by Kylene Hall

---

drivers/char/tpm/tpm_nsc.c +3 -9

@@ -286,8 +286,4 @@ static int __init init_nsc(void)
 	int lo, hi;
 	int nscAddrBase = TPM_ADDR;
 
-	driver_register(&nsc_drv);
-
- 	/* select PM channel 1 */
-	tpm_write_index(nscAddrBase,NSC_LDN_INDEX, 0x12);

@@ -299,6 +297,8 @@ static int __init init_nsc(void)
 			return -ENODEV;
 	}
 
+	driver_register(&nsc_drv);
+
 	hi = tpm_read_index(nscAddrBase, TPM_NSC_BASE0_HI);
 	lo = tpm_read_index(nscAddrBase, TPM_NSC_BASE0_LO);
 	tpm_nsc.base = (hi<<8) | lo;

@@ -306,38 +306,28 @@ static int __init init_nsc(void)
 	/* enable the DPM module */
 	tpm_write_index(nscAddrBase, NSC_LDC_INDEX, 0x01);
 
-	pdev = kmalloc(sizeof(struct platform_device), GFP_KERNEL);
-	if ( !pdev ) 
-		return -ENOMEM;
-
-	memset(pdev, 0, sizeof(struct platform_device));
+	pdev = kzalloc(sizeof(struct platform_device), GFP_KERNEL);

drivers/char/tpm/tpm_nsc.c static int __init init_nsc(void) · line 318

Quoted 10 lines

drivers/char/tpm/tpm_nsc.c +5 -1

+	if ( !pdev ) { 
+		rc = -ENOMEM;
+		goto err_unreg_drv;
+	}
 
 	pdev->name = "tpm_nscl0";
 	pdev->id = -1;
 	pdev->num_resources = 0;
-	pdev->dev.release = tpm_nsc_remove;	
+	pdev->dev.release = tpm_nsc_remove;

This driver is buggy. You must not provide your own release function - it doesn't solve the problem which the warning (which you get when you don't provide one) is telling you about.

You should convert your device driver over to the replacement dynamic platform support, once it is merged. IOW, something like:

        pdev = platform_device_alloc("mydev", id);
        if (pdev) {
                err = platform_device_add_resources(pdev, &resources,
                                                    ARRAY_SIZE(resources));
                if (err == 0)
                        err = platform_device_add_data(pdev, &platform_data,
                                                       sizeof(platform_data));
                if (err == 0)
                        err = platform_device_add(pdev);
        } else {
                err = -ENOMEM;
        }
        if (err)
                platform_device_put(pdev);

Signature · 3 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core