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"?

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"?

> +                     char *offs;
> +
> +                     field_str = strsep(&param, ":");
> +                     offs = strpbrk(field_str, "+-");

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/178875277830.93794.14247844688761142429.stgit@devnote2?part=9

Reply via email to