Re: [PATCH] watchdog/perf: reject empty config before the raw event parse

[Doug Anderson]

Hi,

On Fri, Oct 2, 2026 at 3:17 PM Bradley Morgan <brads@xxxxxxxxxxxxxx> wrote:
>
> On 2 October 2026 23:12:33 BST, Doug Anderson <dianders@xxxxxxxxxxxx>
> wrote:
> >Hi,
> >
> >On Fri, Oct 2, 2026 at 10:46 AM Bradley Morgan <brads@xxxxxxxxxxxxxx>
> >wrote:
> >>
> >> Date: Fri, 2 Oct 2026 17:39:13 +0000
> >> Subject: [PATCH] watchdog/perf: reject empty config before the raw event
> >parse
> >>
> >> Commit 6164be01f179 ("watchdog/perf: optimize bytes copied and remove
> >> manual NUL-termination") replaced strscpy(buf, str, sizeof(buf)) plus a
> >> manual buf[len] = 0 with strscpy(buf, str, len). strscpy() writes
> >> nothing at all when the count is zero, so on a comma directly after
> >> the "r" prefix, nmi_watchdog=r,1 for example, len is 0, buf stays
> >> uninitialized, and kstrtoull() parses whatever stack garbage is there.
> >>
> >> The old code was safe here by accident, strscpy() filled the whole
> >> buffer before buf[len] = 0 overwrote the comma position, so a zero
> >> length just produced an empty string and a clean parse failure.
> >>
> >> Treat an empty config the same as an overlong one and return early.
> >>
> >> Fixes: 6164be01f179 ("watchdog/perf: optimize bytes copied and remove
> >manual NUL-termination")
> >> Signed-off-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>
> >> ---
> >>  kernel/watchdog_perf.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c
> >> index cf05775a96d3..cca0485ba28c 100644
> >> --- a/kernel/watchdog_perf.c
> >> +++ b/kernel/watchdog_perf.c
> >> @@ -301,7 +301,7 @@ void __init hardlockup_config_perf_event(const char
> >*str)
> >>         } else {
> >>                 unsigned int len = comma - str;
> >>
> >> -               if (len > sizeof(buf))
> >> +               if (!len || len > sizeof(buf))
> >>                         return;
> >>
> >>                 strscpy(buf, str, len);
> >
> >I fed your patch to my AI, and it wasn't happy with it. While you fix
> >one corner case, the larger problem is still there.
> >
> >Image if `str` is "300,panic". Then `comma - str` will be 3. Passing 3
> >to `strscpy` will copy at most 2 characters so it has room to use the
> >3rd character as termination. That means `buf` will have "30", not
> >"300". Oops.
> >
> >My AI suggests the correct fix is to change the "len >" to "len >="
> >and then pass "len + 1" to strscpy().
> >
> >if (len >= sizeof(buf))
> >  return;
> >
> >strscpy(buf, str, len + 1);
>
> Makes sense. Do you think you could put this into a separate patch, or
> would you like me to respin? :)

In the end, it's up to Andrew. If it were me, I'd request you respin.
The patch is currently in Andrew's "provisional" tree
(mm-nonmm-unstable), so if you send a v2 he will just drop the old
patch and replace it with your v2.

-Doug