[WATCHDOG] Semi-typical watchdog bug re early misc_register()

It seems that some watchdog drivers are doing following mistake:

	rv = misc_register();
	if (rv < 0)
		return rv;
	rv = request_region();
	if (rv < 0) {
		misc_deregister();
		return rv;
	}

But, right after misc_register() returns, misc device can be opened and
ioctls interacting with hardware issued, and driver can do outb() to
port it doesn't own yet, because request_region() is still pending.

Here is my patch, compile-tested only.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

diff --git a/drivers/char/watchdog/cpu5wdt.c b/drivers/char/watchdog/cpu5wdt.c
index bcd7e36ca0aa67b8f9e6e3c461d2b3f12912d0ab..d0d45a8b09f0a57c379028a4b4e62800c222e223 100644 (file)
--- a/drivers/char/watchdog/cpu5wdt.c
+++ b/drivers/char/watchdog/cpu5wdt.c
@@ -220,17 +220,17 @@ static int __devinit cpu5wdt_init(void)
        if ( verbose )
                printk(KERN_DEBUG PFX "port=0x%x, verbose=%i\n", port, verbose);
 
-       if ( (err = misc_register(&cpu5wdt_misc)) < 0 ) {
-               printk(KERN_ERR PFX "misc_register failed\n");
-               goto no_misc;
-       }
-
        if ( !request_region(port, CPU5WDT_EXTENT, PFX) ) {
                printk(KERN_ERR PFX "request_region failed\n");
                err = -EBUSY;
                goto no_port;
        }
 
+       if ( (err = misc_register(&cpu5wdt_misc)) < 0 ) {
+               printk(KERN_ERR PFX "misc_register failed\n");
+               goto no_misc;
+       }
+
        /* watchdog reboot? */
        val = inb(port + CPU5WDT_STATUS_REG);
        val = (val >> 2) & 1;
@@ -250,9 +250,9 @@ static int __devinit cpu5wdt_init(void)
 
        return 0;
 
-no_port:
-       misc_deregister(&cpu5wdt_misc);
 no_misc:
+       release_region(port, CPU5WDT_EXTENT);
+no_port:
        return err;
 }
 

diff --git a/drivers/char/watchdog/eurotechwdt.c b/drivers/char/watchdog/eurotechwdt.c
index f70387f01b2b1880ffcbb26b9982d2ba68f55f48..b070324e27a6bf903cbe96410f123e24360932fd 100644 (file)
--- a/drivers/char/watchdog/eurotechwdt.c
+++ b/drivers/char/watchdog/eurotechwdt.c
@@ -413,17 +413,10 @@ static int __init eurwdt_init(void)
 {
        int ret;
 
-       ret = misc_register(&eurwdt_miscdev);
-       if (ret) {
-               printk(KERN_ERR "eurwdt: can't misc_register on minor=%d\n",
-               WATCHDOG_MINOR);
-               goto out;
-       }
-
        ret = request_irq(irq, eurwdt_interrupt, IRQF_DISABLED, "eurwdt", NULL);
        if(ret) {
                printk(KERN_ERR "eurwdt: IRQ %d is not free.\n", irq);
-               goto outmisc;
+               goto out;
        }
 
        if (!request_region(io, 2, "eurwdt")) {
@@ -438,6 +431,13 @@ static int __init eurwdt_init(void)
                goto outreg;
        }
 
+       ret = misc_register(&eurwdt_miscdev);
+       if (ret) {
+               printk(KERN_ERR "eurwdt: can't misc_register on minor=%d\n",
+               WATCHDOG_MINOR);
+               goto outreboot;
+       }
+
        eurwdt_unlock_chip();
 
        ret = 0;
@@ -448,14 +448,14 @@ static int __init eurwdt_init(void)
 out:
        return ret;
 
+outreboot:
+       unregister_reboot_notifier(&eurwdt_notifier);
+
 outreg:
        release_region(io, 2);
 
 outirq:
        free_irq(irq, NULL);
-
-outmisc:
-       misc_deregister(&eurwdt_miscdev);
        goto out;
 }
 

diff --git a/drivers/char/watchdog/ibmasr.c b/drivers/char/watchdog/ibmasr.c
index 8195f5023d853c23406f3cd0abe319b68e745d2c..94155f6136c2369a10135cc62b9aef1896755d66 100644 (file)
--- a/drivers/char/watchdog/ibmasr.c
+++ b/drivers/char/watchdog/ibmasr.c
@@ -367,18 +367,17 @@ static int __init ibmasr_init(void)
        if (!asr_type)
                return -ENODEV;
 
+       rc = asr_get_base_address();
+       if (rc)
+               return rc;
+
        rc = misc_register(&asr_miscdev);
        if (rc < 0) {
+               release_region(asr_base, asr_length);
                printk(KERN_ERR PFX "failed to register misc device\n");
                return rc;
        }
 
-       rc = asr_get_base_address();
-       if (rc) {
-               misc_deregister(&asr_miscdev);
-               return rc;
-       }
-
        return 0;
 }
 

diff --git a/drivers/char/watchdog/machzwd.c b/drivers/char/watchdog/machzwd.c
index 76c7fa37fa6c8abdb515bc93beedabbc057687b8..a0d27160c80e1f1e540c3cda4c80a0786dd03b5c 100644 (file)
--- a/drivers/char/watchdog/machzwd.c
+++ b/drivers/char/watchdog/machzwd.c
@@ -440,13 +440,6 @@ static int __init zf_init(void)
        spin_lock_init(&zf_lock);
        spin_lock_init(&zf_port_lock);
 
-       ret = misc_register(&zf_miscdev);
-       if (ret){
-               printk(KERN_ERR "can't misc_register on minor=%d\n",
-                                                       WATCHDOG_MINOR);
-               goto out;
-       }
-
        if(!request_region(ZF_IOBASE, 3, "MachZ ZFL WDT")){
                printk(KERN_ERR "cannot reserve I/O ports at %d\n",
                                                        ZF_IOBASE);
@@ -461,16 +454,23 @@ static int __init zf_init(void)
                goto no_reboot;
        }
 
+       ret = misc_register(&zf_miscdev);
+       if (ret){
+               printk(KERN_ERR "can't misc_register on minor=%d\n",
+                                                       WATCHDOG_MINOR);
+               goto no_misc;
+       }
+
        zf_set_status(0);
        zf_set_control(0);
 
        return 0;
 
+no_misc:
+       unregister_reboot_notifier(&zf_notifier);
 no_reboot:
        release_region(ZF_IOBASE, 3);
 no_region:
-       misc_deregister(&zf_miscdev);
-out:
        return ret;
 }
 

diff --git a/drivers/char/watchdog/sbc8360.c b/drivers/char/watchdog/sbc8360.c
index 67ae42685e752f94d5feb19c7c4684467fa296af..285d85289532da3aadab9c7b0f1e73373fee1d84 100644 (file)
--- a/drivers/char/watchdog/sbc8360.c
+++ b/drivers/char/watchdog/sbc8360.c
@@ -333,18 +333,17 @@ static int __init sbc8360_init(void)
        int res;
        unsigned long int mseconds = 60000;
 
-       spin_lock_init(&sbc8360_lock);
-       res = misc_register(&sbc8360_miscdev);
-       if (res) {
-               printk(KERN_ERR PFX "failed to register misc device\n");
-               goto out_nomisc;
+       if (timeout < 0 || timeout > 63) {
+               printk(KERN_ERR PFX "Invalid timeout index (must be 0-63).\n");
+               res = -EINVAL;
+               goto out;
        }
 
        if (!request_region(SBC8360_ENABLE, 1, "SBC8360")) {
                printk(KERN_ERR PFX "ENABLE method I/O %X is not available.\n",
                       SBC8360_ENABLE);
                res = -EIO;
-               goto out_noenablereg;
+               goto out;
        }
        if (!request_region(SBC8360_BASETIME, 1, "SBC8360")) {
                printk(KERN_ERR PFX
@@ -360,10 +359,11 @@ static int __init sbc8360_init(void)
                goto out_noreboot;
        }
 
-       if (timeout < 0 || timeout > 63) {
-               printk(KERN_ERR PFX "Invalid timeout index (must be 0-63).\n");
-               res = -EINVAL;
-               goto out_noreboot;
+       spin_lock_init(&sbc8360_lock);
+       res = misc_register(&sbc8360_miscdev);
+       if (res) {
+               printk(KERN_ERR PFX "failed to register misc device\n");
+               goto out_nomisc;
        }
 
        wd_margin = wd_times[timeout][0];
@@ -383,13 +383,13 @@ static int __init sbc8360_init(void)
 
        return 0;
 
+      out_nomisc:
+       unregister_reboot_notifier(&sbc8360_notifier);
       out_noreboot:
-       release_region(SBC8360_ENABLE, 1);
        release_region(SBC8360_BASETIME, 1);
-      out_noenablereg:
       out_nobasetimereg:
-       misc_deregister(&sbc8360_miscdev);
-      out_nomisc:
+       release_region(SBC8360_ENABLE, 1);
+      out:
        return res;
 }