Messages in this thread |  | | From | Doug Anderson <> | | Date | Fri, 2 Oct 2026 17:30:05 -0700 | | Subject | Re: [PATCH v2] watchdog/perf: fix off by one in the raw event config copy |
| |
Hi,
On Fri, Oct 2, 2026 at 5:06 PM Bradley Morgan <brads@mainlining.org> 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@chromium.org> > Fixes: 6164be01f179 ("watchdog/perf: optimize bytes copied and remove manual NUL-termination") > Signed-off-by: Bradley Morgan <brads@mainlining.org> > --- > 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
|  |