diff options
author | Alexey Dobriyan <adobriyan@gmail.com> | 2007-03-24 08:58:12 -0400 |
---|---|---|
committer | Wim Van Sebroeck <wim@iguana.be> | 2007-03-26 16:26:11 -0400 |
commit | fb8f7ba077b5c665432082ab205bcd2cb01f6a3c (patch) | |
tree | 13a956eb9a1723bd3d5463230acbfc6513f3b034 | |
parent | 0e94f2ee0d1947ba6c2c00c3e971ff93ce8edec1 (diff) |
[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>
-rw-r--r-- | drivers/char/watchdog/cpu5wdt.c | 14 | ||||
-rw-r--r-- | drivers/char/watchdog/eurotechwdt.c | 22 | ||||
-rw-r--r-- | drivers/char/watchdog/ibmasr.c | 11 | ||||
-rw-r--r-- | drivers/char/watchdog/machzwd.c | 18 | ||||
-rw-r--r-- | drivers/char/watchdog/sbc8360.c | 28 |
5 files changed, 46 insertions, 47 deletions
diff --git a/drivers/char/watchdog/cpu5wdt.c b/drivers/char/watchdog/cpu5wdt.c index bcd7e36ca0aa..d0d45a8b09f0 100644 --- a/drivers/char/watchdog/cpu5wdt.c +++ b/drivers/char/watchdog/cpu5wdt.c | |||
@@ -220,17 +220,17 @@ static int __devinit cpu5wdt_init(void) | |||
220 | if ( verbose ) | 220 | if ( verbose ) |
221 | printk(KERN_DEBUG PFX "port=0x%x, verbose=%i\n", port, verbose); | 221 | printk(KERN_DEBUG PFX "port=0x%x, verbose=%i\n", port, verbose); |
222 | 222 | ||
223 | if ( (err = misc_register(&cpu5wdt_misc)) < 0 ) { | ||
224 | printk(KERN_ERR PFX "misc_register failed\n"); | ||
225 | goto no_misc; | ||
226 | } | ||
227 | |||
228 | if ( !request_region(port, CPU5WDT_EXTENT, PFX) ) { | 223 | if ( !request_region(port, CPU5WDT_EXTENT, PFX) ) { |
229 | printk(KERN_ERR PFX "request_region failed\n"); | 224 | printk(KERN_ERR PFX "request_region failed\n"); |
230 | err = -EBUSY; | 225 | err = -EBUSY; |
231 | goto no_port; | 226 | goto no_port; |
232 | } | 227 | } |
233 | 228 | ||
229 | if ( (err = misc_register(&cpu5wdt_misc)) < 0 ) { | ||
230 | printk(KERN_ERR PFX "misc_register failed\n"); | ||
231 | goto no_misc; | ||
232 | } | ||
233 | |||
234 | /* watchdog reboot? */ | 234 | /* watchdog reboot? */ |
235 | val = inb(port + CPU5WDT_STATUS_REG); | 235 | val = inb(port + CPU5WDT_STATUS_REG); |
236 | val = (val >> 2) & 1; | 236 | val = (val >> 2) & 1; |
@@ -250,9 +250,9 @@ static int __devinit cpu5wdt_init(void) | |||
250 | 250 | ||
251 | return 0; | 251 | return 0; |
252 | 252 | ||
253 | no_port: | ||
254 | misc_deregister(&cpu5wdt_misc); | ||
255 | no_misc: | 253 | no_misc: |
254 | release_region(port, CPU5WDT_EXTENT); | ||
255 | no_port: | ||
256 | return err; | 256 | return err; |
257 | } | 257 | } |
258 | 258 | ||
diff --git a/drivers/char/watchdog/eurotechwdt.c b/drivers/char/watchdog/eurotechwdt.c index f70387f01b2b..b070324e27a6 100644 --- a/drivers/char/watchdog/eurotechwdt.c +++ b/drivers/char/watchdog/eurotechwdt.c | |||
@@ -413,17 +413,10 @@ static int __init eurwdt_init(void) | |||
413 | { | 413 | { |
414 | int ret; | 414 | int ret; |
415 | 415 | ||
416 | ret = misc_register(&eurwdt_miscdev); | ||
417 | if (ret) { | ||
418 | printk(KERN_ERR "eurwdt: can't misc_register on minor=%d\n", | ||
419 | WATCHDOG_MINOR); | ||
420 | goto out; | ||
421 | } | ||
422 | |||
423 | ret = request_irq(irq, eurwdt_interrupt, IRQF_DISABLED, "eurwdt", NULL); | 416 | ret = request_irq(irq, eurwdt_interrupt, IRQF_DISABLED, "eurwdt", NULL); |
424 | if(ret) { | 417 | if(ret) { |
425 | printk(KERN_ERR "eurwdt: IRQ %d is not free.\n", irq); | 418 | printk(KERN_ERR "eurwdt: IRQ %d is not free.\n", irq); |
426 | goto outmisc; | 419 | goto out; |
427 | } | 420 | } |
428 | 421 | ||
429 | if (!request_region(io, 2, "eurwdt")) { | 422 | if (!request_region(io, 2, "eurwdt")) { |
@@ -438,6 +431,13 @@ static int __init eurwdt_init(void) | |||
438 | goto outreg; | 431 | goto outreg; |
439 | } | 432 | } |
440 | 433 | ||
434 | ret = misc_register(&eurwdt_miscdev); | ||
435 | if (ret) { | ||
436 | printk(KERN_ERR "eurwdt: can't misc_register on minor=%d\n", | ||
437 | WATCHDOG_MINOR); | ||
438 | goto outreboot; | ||
439 | } | ||
440 | |||
441 | eurwdt_unlock_chip(); | 441 | eurwdt_unlock_chip(); |
442 | 442 | ||
443 | ret = 0; | 443 | ret = 0; |
@@ -448,14 +448,14 @@ static int __init eurwdt_init(void) | |||
448 | out: | 448 | out: |
449 | return ret; | 449 | return ret; |
450 | 450 | ||
451 | outreboot: | ||
452 | unregister_reboot_notifier(&eurwdt_notifier); | ||
453 | |||
451 | outreg: | 454 | outreg: |
452 | release_region(io, 2); | 455 | release_region(io, 2); |
453 | 456 | ||
454 | outirq: | 457 | outirq: |
455 | free_irq(irq, NULL); | 458 | free_irq(irq, NULL); |
456 | |||
457 | outmisc: | ||
458 | misc_deregister(&eurwdt_miscdev); | ||
459 | goto out; | 459 | goto out; |
460 | } | 460 | } |
461 | 461 | ||
diff --git a/drivers/char/watchdog/ibmasr.c b/drivers/char/watchdog/ibmasr.c index 8195f5023d85..94155f6136c2 100644 --- a/drivers/char/watchdog/ibmasr.c +++ b/drivers/char/watchdog/ibmasr.c | |||
@@ -367,18 +367,17 @@ static int __init ibmasr_init(void) | |||
367 | if (!asr_type) | 367 | if (!asr_type) |
368 | return -ENODEV; | 368 | return -ENODEV; |
369 | 369 | ||
370 | rc = asr_get_base_address(); | ||
371 | if (rc) | ||
372 | return rc; | ||
373 | |||
370 | rc = misc_register(&asr_miscdev); | 374 | rc = misc_register(&asr_miscdev); |
371 | if (rc < 0) { | 375 | if (rc < 0) { |
376 | release_region(asr_base, asr_length); | ||
372 | printk(KERN_ERR PFX "failed to register misc device\n"); | 377 | printk(KERN_ERR PFX "failed to register misc device\n"); |
373 | return rc; | 378 | return rc; |
374 | } | 379 | } |
375 | 380 | ||
376 | rc = asr_get_base_address(); | ||
377 | if (rc) { | ||
378 | misc_deregister(&asr_miscdev); | ||
379 | return rc; | ||
380 | } | ||
381 | |||
382 | return 0; | 381 | return 0; |
383 | } | 382 | } |
384 | 383 | ||
diff --git a/drivers/char/watchdog/machzwd.c b/drivers/char/watchdog/machzwd.c index 76c7fa37fa6c..a0d27160c80e 100644 --- a/drivers/char/watchdog/machzwd.c +++ b/drivers/char/watchdog/machzwd.c | |||
@@ -440,13 +440,6 @@ static int __init zf_init(void) | |||
440 | spin_lock_init(&zf_lock); | 440 | spin_lock_init(&zf_lock); |
441 | spin_lock_init(&zf_port_lock); | 441 | spin_lock_init(&zf_port_lock); |
442 | 442 | ||
443 | ret = misc_register(&zf_miscdev); | ||
444 | if (ret){ | ||
445 | printk(KERN_ERR "can't misc_register on minor=%d\n", | ||
446 | WATCHDOG_MINOR); | ||
447 | goto out; | ||
448 | } | ||
449 | |||
450 | if(!request_region(ZF_IOBASE, 3, "MachZ ZFL WDT")){ | 443 | if(!request_region(ZF_IOBASE, 3, "MachZ ZFL WDT")){ |
451 | printk(KERN_ERR "cannot reserve I/O ports at %d\n", | 444 | printk(KERN_ERR "cannot reserve I/O ports at %d\n", |
452 | ZF_IOBASE); | 445 | ZF_IOBASE); |
@@ -461,16 +454,23 @@ static int __init zf_init(void) | |||
461 | goto no_reboot; | 454 | goto no_reboot; |
462 | } | 455 | } |
463 | 456 | ||
457 | ret = misc_register(&zf_miscdev); | ||
458 | if (ret){ | ||
459 | printk(KERN_ERR "can't misc_register on minor=%d\n", | ||
460 | WATCHDOG_MINOR); | ||
461 | goto no_misc; | ||
462 | } | ||
463 | |||
464 | zf_set_status(0); | 464 | zf_set_status(0); |
465 | zf_set_control(0); | 465 | zf_set_control(0); |
466 | 466 | ||
467 | return 0; | 467 | return 0; |
468 | 468 | ||
469 | no_misc: | ||
470 | unregister_reboot_notifier(&zf_notifier); | ||
469 | no_reboot: | 471 | no_reboot: |
470 | release_region(ZF_IOBASE, 3); | 472 | release_region(ZF_IOBASE, 3); |
471 | no_region: | 473 | no_region: |
472 | misc_deregister(&zf_miscdev); | ||
473 | out: | ||
474 | return ret; | 474 | return ret; |
475 | } | 475 | } |
476 | 476 | ||
diff --git a/drivers/char/watchdog/sbc8360.c b/drivers/char/watchdog/sbc8360.c index 67ae42685e75..285d85289532 100644 --- a/drivers/char/watchdog/sbc8360.c +++ b/drivers/char/watchdog/sbc8360.c | |||
@@ -333,18 +333,17 @@ static int __init sbc8360_init(void) | |||
333 | int res; | 333 | int res; |
334 | unsigned long int mseconds = 60000; | 334 | unsigned long int mseconds = 60000; |
335 | 335 | ||
336 | spin_lock_init(&sbc8360_lock); | 336 | if (timeout < 0 || timeout > 63) { |
337 | res = misc_register(&sbc8360_miscdev); | 337 | printk(KERN_ERR PFX "Invalid timeout index (must be 0-63).\n"); |
338 | if (res) { | 338 | res = -EINVAL; |
339 | printk(KERN_ERR PFX "failed to register misc device\n"); | 339 | goto out; |
340 | goto out_nomisc; | ||
341 | } | 340 | } |
342 | 341 | ||
343 | if (!request_region(SBC8360_ENABLE, 1, "SBC8360")) { | 342 | if (!request_region(SBC8360_ENABLE, 1, "SBC8360")) { |
344 | printk(KERN_ERR PFX "ENABLE method I/O %X is not available.\n", | 343 | printk(KERN_ERR PFX "ENABLE method I/O %X is not available.\n", |
345 | SBC8360_ENABLE); | 344 | SBC8360_ENABLE); |
346 | res = -EIO; | 345 | res = -EIO; |
347 | goto out_noenablereg; | 346 | goto out; |
348 | } | 347 | } |
349 | if (!request_region(SBC8360_BASETIME, 1, "SBC8360")) { | 348 | if (!request_region(SBC8360_BASETIME, 1, "SBC8360")) { |
350 | printk(KERN_ERR PFX | 349 | printk(KERN_ERR PFX |
@@ -360,10 +359,11 @@ static int __init sbc8360_init(void) | |||
360 | goto out_noreboot; | 359 | goto out_noreboot; |
361 | } | 360 | } |
362 | 361 | ||
363 | if (timeout < 0 || timeout > 63) { | 362 | spin_lock_init(&sbc8360_lock); |
364 | printk(KERN_ERR PFX "Invalid timeout index (must be 0-63).\n"); | 363 | res = misc_register(&sbc8360_miscdev); |
365 | res = -EINVAL; | 364 | if (res) { |
366 | goto out_noreboot; | 365 | printk(KERN_ERR PFX "failed to register misc device\n"); |
366 | goto out_nomisc; | ||
367 | } | 367 | } |
368 | 368 | ||
369 | wd_margin = wd_times[timeout][0]; | 369 | wd_margin = wd_times[timeout][0]; |
@@ -383,13 +383,13 @@ static int __init sbc8360_init(void) | |||
383 | 383 | ||
384 | return 0; | 384 | return 0; |
385 | 385 | ||
386 | out_nomisc: | ||
387 | unregister_reboot_notifier(&sbc8360_notifier); | ||
386 | out_noreboot: | 388 | out_noreboot: |
387 | release_region(SBC8360_ENABLE, 1); | ||
388 | release_region(SBC8360_BASETIME, 1); | 389 | release_region(SBC8360_BASETIME, 1); |
389 | out_noenablereg: | ||
390 | out_nobasetimereg: | 390 | out_nobasetimereg: |
391 | misc_deregister(&sbc8360_miscdev); | 391 | release_region(SBC8360_ENABLE, 1); |
392 | out_nomisc: | 392 | out: |
393 | return res; | 393 | return res; |
394 | } | 394 | } |
395 | 395 | ||