[PATCH] watchdog: Differentiate scenarios when watchdog is closed

[PATCH] watchdog: Differentiate scenarios when watchdog is closed

[Charles Haithcock]

Presently, when a watchdog device is closed, we print "watchdog did not
stop" in a few different scenarios;

1. When nowayout is set
2. When the watchdog is able to close, has received the magic character
   to stop, but fails to close in device-specific code paths
3. When userspace deliberately closes it without stopping it

For 1, we explicitly print we can not close because of nowayout. Nothing
differentiates the other two however.

This change adds a print to indicate the watchdog was closed while still
running.

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Charles Haithcock <chaithco@redhat.com>
---

v1. Introduce change and modify logic to differentiate
v2. Simplify but still differentiate

 drivers/watchdog/watchdog_dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index d7895009a2..937bdd4c92 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -305,6 +305,10 @@ static int watchdog_stop(struct watchdog_device *wdd)
 	if (wdd->ops->stop) {
 		clear_bit(WDOG_HW_RUNNING, &wdd->status);
 		err = wdd->ops->stop(wdd);
+		if (err < 0) {
+			pr_err("watchdog%d: Failed to stop watchdog: %pe\n", wdd->id,
+				ERR_PTR(err));
+		}
 		trace_watchdog_stop(wdd, err);
 	} else {
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
-- 
2.55.0

Re: [PATCH] watchdog: Differentiate scenarios when watchdog is closed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If `wdd->ops->stop()` fails, `WDOG_HW_RUNNING` is never restored, bypassing module unload protections and breaking fallback keepalive pings, leading to a Use-After-Free.
--

commit aaf821e8a7620610688b1a164fc083439874422c
Author: Charles Haithcock <chaithco@redhat.com>

watchdog: Differentiate scenarios when watchdog is closed

This patch adds an error message when the watchdog device fails to stop in
the device-specific code paths. This helps differentiate between scenarios
where the watchdog was intentionally left running (such as when nowayout is
set) versus when it simply failed to close.

> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 9a5e544b886bf..6e3e437b7851f 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -305,6 +305,10 @@ static int watchdog_stop(struct watchdog_device *wdd)
>  	if (wdd->ops->stop) {
>  		clear_bit(WDOG_HW_RUNNING, &wdd->status);
>  		err = wdd->ops->stop(wdd);
> +		if (err < 0) {

[Severity: High]
This is a pre-existing issue, but if wdd->ops->stop() returns an error, is
WDOG_HW_RUNNING left incorrectly cleared?

It appears clear_bit(WDOG_HW_RUNNING, &wdd->status) is called before
wdd->ops->stop(). If the stop callback fails (e.g., an I2C failure in
drivers like ziirave), WDOG_HW_RUNNING remains cleared while WDOG_ACTIVE
remains set.

Could this bypass module unload protections and lead to a Use-After-Free?

If userspace closes the device and watchdog_stop() fails,
watchdog_release() will execute:

drivers/watchdog/watchdog_dev.c:watchdog_release() {
        ...
        if (err < 0) {
                pr_crit("watchdog%d: watchdog did not stop!\n", wdd->id);
                watchdog_ping(wdd);
        }
        ...
}

Because WDOG_HW_RUNNING is cleared, watchdog_hw_running() evaluates to
false and the keepalive watchdog_ping() becomes a no-op.

Later in watchdog_release(), it evaluates:

drivers/watchdog/watchdog_dev.c:watchdog_release() {
        ...
        running = wdd && watchdog_hw_running(wdd);
        mutex_unlock(&wd_data->lock);

        if (!running) {
                module_put(wd_data->cdev.owner);
                put_device(&wd_data->dev);
        }
        ...
}

Since running is false, it drops module references. This permits the
module to be unloaded and watchdog_core_data to be freed, but the
background worker watchdog_update_worker() remains active because
WDOG_ACTIVE is still set. Will this cause a kernel panic when the hrtimer
expires and accesses the freed struct or unloaded module?

> +			pr_err("watchdog%d: Failed to stop watchdog: %pe\n", wdd->id,
> +				ERR_PTR(err));
> +		}
>  		trace_watchdog_stop(wdd, err);
>  	} else {
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901214251.760184-1-chaithco@redhat.com?part=1

Re: [PATCH] watchdog: Differentiate scenarios when watchdog is closed

[Guenter Roeck]

On Tue, Sep 01, 2026 at 03:42:49PM -0600, Charles Haithcock wrote:
> Presently, when a watchdog device is closed, we print "watchdog did not
> stop" in a few different scenarios;
> 
> 1. When nowayout is set
> 2. When the watchdog is able to close, has received the magic character
>    to stop, but fails to close in device-specific code paths
> 3. When userspace deliberately closes it without stopping it
> 
> For 1, we explicitly print we can not close because of nowayout. Nothing
> differentiates the other two however.
> 
> This change adds a print to indicate the watchdog was closed while still
> running.
> 
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Charles Haithcock <chaithco@redhat.com>

Applied, but please version your patches in the future.

Thanks,
Guenter

> ---
> 
> v1. Introduce change and modify logic to differentiate
> v2. Simplify but still differentiate
> 
>  drivers/watchdog/watchdog_dev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index d7895009a2..937bdd4c92 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -305,6 +305,10 @@ static int watchdog_stop(struct watchdog_device *wdd)
>  	if (wdd->ops->stop) {
>  		clear_bit(WDOG_HW_RUNNING, &wdd->status);
>  		err = wdd->ops->stop(wdd);
> +		if (err < 0) {
> +			pr_err("watchdog%d: Failed to stop watchdog: %pe\n", wdd->id,
> +				ERR_PTR(err));
> +		}
>  		trace_watchdog_stop(wdd, err);
>  	} else {
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);

Re: [PATCH] watchdog: Differentiate scenarios when watchdog is closed

[chaithco]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On Tue, 2026-09-01 at 16:02 -0700, Guenter Roeck wrote:
> 
> Applied, but please version your patches in the future.
> 
Ah thank you so much for the feedback! I am a fair bit newer to
contributing upstream; I presumed (erroneously) adding the versioning
after --- was sufficient. I will make sure to add the version number to
the commit message in the future as well!

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]