Linux Watchdog driver development
 help / color / mirror / Atom feed
From: Tzung-Bi Shih <tzungbi@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: linux-watchdog@vger.kernel.org
Subject: Re: [PATCH] watchdog: pretimeout: Add "dump" pretimeout governor
Date: Tue, 7 Jul 2026 10:28:07 +0000	[thread overview]
Message-ID: <akzUtxGzCslsJL1l@google.com> (raw)
In-Reply-To: <20260707080225.4C52D1F000E9@smtp.kernel.org>

On Tue, Jul 07, 2026 at 08:02:24AM +0000, sashiko-bot@kernel.org wrote:
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 08cb8612d41fe..7b291ea8b0f01 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> 
> [ ... ]
> 
> > @@ -93,10 +93,19 @@ config WATCHDOG_PRETIMEOUT_GOV_SEL
> >  
> >  if WATCHDOG_PRETIMEOUT_GOV
> >  
> > +config WATCHDOG_PRETIMEOUT_GOV_DUMP
> > +	tristate "Dump watchdog pretimeout governor"
> 
> [Severity: Low]
> Will this fail to build if compiled as a module?
> 
> When CONFIG_WATCHDOG_PRETIMEOUT_GOV_DUMP=m, trigger_all_cpu_backtrace() can
> rely on unexported architecture-specific functions (such as
> arch_trigger_cpumask_backtrace() on x86, ARM, and ARM64), which causes a
> modpost build failure.

Use a bool instead in v2 [1].

[1] https://lore.kernel.org/all/20260707102105.3600275-1-tzungbi@kernel.org

> > diff --git a/drivers/watchdog/pretimeout_dump.c b/drivers/watchdog/pretimeout_dump.c
> > new file mode 100644
> > index 0000000000000..c5d3dac2606c4
> > --- /dev/null
> > +++ b/drivers/watchdog/pretimeout_dump.c
> 
> [ ... ]
> 
> > +static void __exit watchdog_gov_dump_unregister(void)
> > +{
> > +	watchdog_unregister_governor(&watchdog_gov_dump);
> > +}
> 
> [Severity: High]
> This is a pre-existing issue, but does unregistering the default watchdog
> pretimeout governor leave dangling pointers?
> 
> If this module is set as the default governor and then unloaded,
> watchdog_unregister_governor() attempts to reset the active governors:
> 
>     list_for_each_entry(p, &pretimeout_list, entry)
>         if (p->wdd->gov == gov)
>             p->wdd->gov = default_gov;
> 
> If the unregistered governor is the default governor, default_gov equals
> gov. This leaves p->wdd->gov pointing to the unloaded module's struct.
> 
> Could this lead to a use-after-free upon the next pretimeout event or sysfs
> read?

Proposed a fix in [2].

[2] https://lore.kernel.org/all/20260707101803.3598173-1-tzungbi@kernel.org

      reply	other threads:[~2026-07-07 10:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  7:31 [PATCH] watchdog: pretimeout: Add "dump" pretimeout governor Tzung-Bi Shih
2026-07-07  8:02 ` sashiko-bot
2026-07-07 10:28   ` Tzung-Bi Shih [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akzUtxGzCslsJL1l@google.com \
    --to=tzungbi@kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox