All of lore.kernel.org
 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 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.