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

[Bradley Morgan]

On 3 October 2026 01:30:05 BST, Doug Anderson <dianders@xxxxxxxxxxxx>
wrote:
>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
>

Thanks, god sake, I did a fart, I'll resend V2 in a new thread... (Sigh,
new email tool bugging me)
--- Thanks!
"I'm not a very positive person" - Linus torvalds