On Mon, 07 Sep 2026 04:10:36 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] clear_wprobe incorrectly rejects legitimate fields starting with > "unlimited" > -- > > commit f87f4797ae7a52cd2de4840afea59fb8bb8bbaf6 > Author: Masami Hiramatsu (Google) <[email protected]> > > tracing/wprobe: Add set_wprobe and clear_wprobe event triggers > > This commit adds the set_wprobe and clear_wprobe event triggers to dynamically > attach and detach hardware breakpoint address monitoring based on event field > contents. > > Link: > https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao...@gmail.com/ > > > diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c > > --- a/kernel/trace/trace_wprobe.c > > +++ b/kernel/trace/trace_wprobe.c > > [ ... ] > > > +static int wprobe_trigger_cmd_parse(struct event_command *cmd_ops, > > + struct trace_event_file *file, > > + char *glob, char *cmd, > > + char *param_and_filter) > > +{ > > [ ... ] > > > + /* clear_wprobe does not need field, but can have optional field. */ > > + if (!clear) { > > [ ... ] > > > + } else if (param && (isalpha(param[0]) || param[0] == '_')) { > > + if (strncmp(param, "count=", 6) != 0 && > > + strncmp(param, "unlimited", 9) != 0) { > [Severity: Medium] > Will this substring comparison mistakenly intercept legitimate C-identifier > field names that start with "unlimited"?
Ah, yes. I understand it. Maybe we can use strcmp(param, "unlimited") so that it will pass the field name that starts with "unlimited". > > If a user passes a legitimate event field name like "unlimited_size" to > clear_wprobe, the strncmp(param, "unlimited", 9) != 0 check will evaluate > to false. The parsing logic in wprobe_trigger_cmd_parse() will mistakenly > assume it is a count parameter, skip the field parsing, and pass the string > to event_trigger_parse_num(). > > Since event_trigger_parse_num() attempts to parse "unlimited_size" using > kstrtoul(), it will fail and return -EINVAL. Could this prevent users from > using clear_wprobe on any valid field beginning with "unlimited"? Thanks, -- Masami Hiramatsu (Google) <[email protected]>
