From: Alexey Dobriyan Date: Sat, 24 Mar 2007 12:58:12 +0000 (+0300) Subject: [WATCHDOG] Semi-typical watchdog bug re early misc_register() X-Git-Tag: v3.12-rc1~29566^2~9 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fb8f7ba077b5c665432082ab205bcd2cb01f6a3c;p=kernel%2Fkernel-generic.git [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 Signed-off-by: Wim Van Sebroeck --- diff --git a/drivers/char/watchdog/cpu5wdt.c b/drivers/char/watchdog/cpu5wdt.c index bcd7e36..d0d45a8 100644 --- 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 f70387f..b070324 100644 --- 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 8195f50..94155f6 100644 --- 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 76c7fa3..a0d2716 100644 --- 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 67ae426..285d852 100644 --- 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; }