All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor()
@ 2026-07-07 10:18 Tzung-Bi Shih
  2026-07-07 10:29 ` sashiko-bot
  2026-07-08 14:06 ` Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Tzung-Bi Shih @ 2026-07-07 10:18 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck; +Cc: linux-watchdog, linux-kernel, tzungbi

When a watchdog governor is unregistered, it updates existing watchdog
devices that were using this governor by falling back to `default_gov`.

If the governor being unregistered is currently set as `default_gov`,
the `default_gov` is never cleared.  This leads to 2 use-after-free
issues:
1. New watchdog devices registered after this point will inherit the
   dangling `default_gov`.
2. Existing watchdog devices using the unregistered governor will have
   their `wdd->gov` reassigned to the dangling `default_gov`.

Fix the UAF by clearing `default_gov` if it matches the governor being
unregistered.

Fixes: da0d12ff2b82 ("watchdog: pretimeout: add panic pretimeout governor")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/watchdog/watchdog_pretimeout.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index 19eb2ed2c7cb..02e09b9e396d 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -167,6 +167,8 @@ void watchdog_unregister_governor(struct watchdog_governor *gov)
 	}
 
 	spin_lock_irq(&pretimeout_lock);
+	if (default_gov == gov)
+		default_gov = NULL;
 	list_for_each_entry(p, &pretimeout_list, entry)
 		if (p->wdd->gov == gov)
 			p->wdd->gov = default_gov;
-- 
2.55.0.rc2.803.g1fd1e6609c-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor()
  2026-07-07 10:18 [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor() Tzung-Bi Shih
@ 2026-07-07 10:29 ` sashiko-bot
  2026-07-08 14:06 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-07 10:29 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: linux-watchdog

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

Pre-existing issues:
- [High] A teardown symmetry violation in `watchdog_dev_unregister()` exposes a race window with sysfs, allowing a Use-After-Free of governor modules.
- [Medium] A sysfs race condition during device registration allows user-configured pretimeout governors to be silently overwritten by `default_gov`.
--

--- Patch [1]: [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor() ---
commit f31dd8ccfc4553641fb99ced744ddc61a7ad970c
Author: Tzung-Bi Shih <tzungbi@kernel.org>

watchdog: pretimeout: Fix UAF in watchdog_unregister_governor()

When a watchdog governor is unregistered, it updates existing watchdog
devices using it to fall back to default_gov. This commit clears
default_gov if it matches the governor being unregistered to prevent
two use-after-free scenarios where a dangling pointer is inherited.

> diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
> index 19eb2ed2c7cb0..02e09b9e396da 100644
> --- a/drivers/watchdog/watchdog_pretimeout.c
> +++ b/drivers/watchdog/watchdog_pretimeout.c
> @@ -167,6 +167,8 @@ void watchdog_unregister_governor(struct watchdog_governor *gov)
>  	}
>  
>  	spin_lock_irq(&pretimeout_lock);
> +	if (default_gov == gov)
> +		default_gov = NULL;
>  	list_for_each_entry(p, &pretimeout_list, entry)

[Severity: High]
This is a pre-existing issue, but is there a teardown symmetry violation in
watchdog_dev_unregister() that exposes a race window with sysfs?

If watchdog_dev_unregister() calls watchdog_unregister_pretimeout() before
watchdog_cdev_unregister():

drivers/watchdog/watchdog_dev.c:watchdog_dev_unregister() {
	watchdog_unregister_pretimeout(wdd);
	watchdog_cdev_unregister(wdd);
}

The device is removed from pretimeout_list, but the sysfs interface is still
active. A concurrent write to the pretimeout_governor sysfs node can
successfully assign a governor to wdd->gov:

drivers/watchdog/watchdog_pretimeout.c:watchdog_pretimeout_governor_set() {
	spin_lock_irq(&pretimeout_lock);
	wdd->gov = priv->gov;
	spin_unlock_irq(&pretimeout_lock);
}

If the governor module is then unloaded, watchdog_unregister_governor() fails
to clear wdd->gov because the device is no longer in pretimeout_list. If an
in-flight hardware watchdog pretimeout interrupt fires,
watchdog_notify_pretimeout() dereferences the dangling wdd->gov pointer:

drivers/watchdog/watchdog_pretimeout.c:watchdog_notify_pretimeout() {
	wdd->gov->pretimeout(wdd);
}

Can this lead to a use-after-free?

>  		if (p->wdd->gov == gov)
>  			p->wdd->gov = default_gov;

[Severity: Medium]
This is also a pre-existing issue, but does watchdog_dev_register() silently
overwrite user-configured pretimeout governors due to a sysfs race condition?

When watchdog_dev_register() is called, it registers the character device and
sysfs interfaces before fully initializing the internal pretimeout state:

drivers/watchdog/watchdog_dev.c:watchdog_dev_register() {
	ret = watchdog_cdev_register(wdd);
	if (ret)
		return ret;

	ret = watchdog_register_pretimeout(wdd);
}

If a userspace udev rule or concurrent process immediately writes to the
pretimeout_governor sysfs node, setting wdd->gov, the subsequent call to
watchdog_register_pretimeout() unconditionally overwrites it with default_gov
without synchronizing with sysfs:

drivers/watchdog/watchdog_pretimeout.c:watchdog_register_pretimeout() {
	spin_lock_irq(&pretimeout_lock);
	list_add(&p->entry, &pretimeout_list);
	p->wdd = wdd;
	wdd->gov = default_gov;
	spin_unlock_irq(&pretimeout_lock);
}

Could this result in the user's requested governor being silently ignored?

>  	spin_unlock_irq(&pretimeout_lock);
>  
>  	mutex_unlock(&governor_lock);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707101803.3598173-1-tzungbi@kernel.org?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor()
  2026-07-07 10:18 [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor() Tzung-Bi Shih
  2026-07-07 10:29 ` sashiko-bot
@ 2026-07-08 14:06 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2026-07-08 14:06 UTC (permalink / raw)
  To: Tzung-Bi Shih; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel

On Tue, Jul 07, 2026 at 10:18:03AM +0000, Tzung-Bi Shih wrote:
> When a watchdog governor is unregistered, it updates existing watchdog
> devices that were using this governor by falling back to `default_gov`.
> 
> If the governor being unregistered is currently set as `default_gov`,
> the `default_gov` is never cleared.  This leads to 2 use-after-free
> issues:
> 1. New watchdog devices registered after this point will inherit the
>    dangling `default_gov`.
> 2. Existing watchdog devices using the unregistered governor will have
>    their `wdd->gov` reassigned to the dangling `default_gov`.
> 
> Fix the UAF by clearing `default_gov` if it matches the governor being
> unregistered.
> 
> Fixes: da0d12ff2b82 ("watchdog: pretimeout: add panic pretimeout governor")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Applied.

Thanks,
Guenter

> ---
>  drivers/watchdog/watchdog_pretimeout.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
> index 19eb2ed2c7cb..02e09b9e396d 100644
> --- a/drivers/watchdog/watchdog_pretimeout.c
> +++ b/drivers/watchdog/watchdog_pretimeout.c
> @@ -167,6 +167,8 @@ void watchdog_unregister_governor(struct watchdog_governor *gov)
>  	}
>  
>  	spin_lock_irq(&pretimeout_lock);
> +	if (default_gov == gov)
> +		default_gov = NULL;
>  	list_for_each_entry(p, &pretimeout_list, entry)
>  		if (p->wdd->gov == gov)
>  			p->wdd->gov = default_gov;

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-08 14:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 10:18 [PATCH] watchdog: pretimeout: Fix UAF in watchdog_unregister_governor() Tzung-Bi Shih
2026-07-07 10:29 ` sashiko-bot
2026-07-08 14:06 ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.