Re: [PATCH v15 09/12] tracing/wprobe: Add set_wprobe and clear_wprobe event triggers

[sashiko-bot@kernel.org]

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) <mhiramat@kernel.org>

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.wangjinchao600@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