Re: [PATCH v2] watchdog/perf: fix off by one in the raw event config copy

[Doug Anderson]

Hi,

On Fri, Oct 2, 2026 at 5:06 PM Bradley Morgan <brads@xxxxxxxxxxxxxx> wrote:
>
> You were right, the truncation was the bigger half of the bug and my
> v1 only closed the corner. Fixed both with your len + 1 shape.

The above should have been "after the cut". Where you have it now,
it'll end up in the commit message.


> 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). That count is one less
> than the code needs, strscpy() always reserves the last byte of the
> destination for the NUL, so the config loses its final digit.
> nmi_watchdog=r300,panic for example ends up with buf = "30" and arms
> the raw event with the wrong config.
>
> The same count also drops the empty case on the floor, strscpy() with
> a zero count writes nothing at all, so nmi_watchdog=r,1 leaves buf
> uninitialized and kstrtoull() reads stack garbage.
>
> The old code was safe on both counts by accident, strscpy() filled the
> whole buffer before buf[len] = 0 overwrote the comma position, so the
> worst outcome was a truncated parse failure.
>
> Pass len + 1 so the copy includes the character the NUL replaces, and
> reject len >= sizeof(buf) like the code did before the optimization,
> which also makes an empty config a clean parse failure again.
>
> Suggested-by: Doug Anderson <dianders@xxxxxxxxxxxx>
> Fixes: 6164be01f179 ("watchdog/perf: optimize bytes copied and remove manual NUL-termination")
> Signed-off-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>
> ---
>  kernel/watchdog_perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c
> index cca0485ba28c..a4f677c16b20 100644
> --- a/kernel/watchdog_perf.c
> +++ b/kernel/watchdog_perf.c
> @@ -301,10 +301,10 @@ void __init hardlockup_config_perf_event(const char *str)
>         } else {
>                 unsigned int len = comma - str;
>
> -               if (!len || len > sizeof(buf))
> +               if (len >= sizeof(buf))

It looks like you sent this atop your previous v1 patch. You should
send your v2 patch as if your v1 patch wasn't applied.

Also, you probably don't need my Suggested-by tag. I just gave review
feedback on v1, which usually doesn't warrant a Suggested-by.

-Doug