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

[Bradley Morgan]

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? :)

>
>-Doug
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds