Hi all, I recently got to look into the cpu5wdt driver to backport a security fix. This driver seems to be in a pretty sorry state. 1* The driver accesses arbitrary I/O ports without ever checking which hardware it runs on. I have no idea what this "sma cpu5" thing is, a quick web search did not return anything relevant, only that sma.de still exists but nothing about "CPU5" specifically. The Kconfig description is "TBD" (not joking...). I can only imagine this is a very specific, very old and probably no longer relevant piece of hardware. The driver comes with zero documentation. I tried to contact the original author but did not get any answer. 2* There's no explanation of what cpu5wdt_lock is supposed to protect, but its use throughout the driver is suspiciously inconsistent. In most places, the lock is held when accessing cpu5wdt_device.running, ticks, cpu5wdt_device.queue, cpu5wdt_device.timer and cpu5wdt_device.stop. However there are exceptions to that, with cpu5wdt_trigger() using cpu5wdt_device.running and ticks without holding the lock. Likewise, cpu5wdt_exit() is using cpu5wdt_device.queue, cpu5wdt_device.stop and cpu5wdt_device.timer without holding the lock, which could race with cpu5wdt_trigger(). cpu5wdt_reset() is also touching ticks without holding the lock. Not sure how atomic "ticks--" is guaranteed to be on X86, if atomicity isn't fully guaranteed then this is racy (FWIW ticks was originally declared volatile, but volatile was removed to silent a warning). 3* The driver doesn't implement WDIOC_SETTIMEOUT. The timeout is set at module load time as an arbitrary tick count. The default is 10000 and the driver is ticking every (HZ/10+1) jiffies, which leads to an inaccurate and undocumented 16 minutes 40 seconds timeout, after which the driver stops pinging the hardware but we have no idea how long it will take before the hardware watchdog actually reboots the system. This is clearly incompatible with the expectations of modern software stacks (systemd, high-availability setups). 4* I also found a potential integer overflow in cpu5wdt_start, using WDIOS_ENABLECARD 2^64 times without closing the device would "stop the watchdog", so the system would not reboot even if the watchdog process stops writing to the device. Not a serious issue as nobody sane would do that, but still... The driver was added in 2003 and there's no evidence that it has any recent user, all changes seem to be tree-wide, subsystem-wide, or the result of static code analysis. Would anyone object to the plain removal of driver cpu5wdt from the kernel tree? Thanks, -- Jean Delvare SUSE L3 Support
