Re: [PATCH v3 01/16] tracing/filters: Fix regex_match_end() for zero-length strings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing/filters: redundant check in regex_match_end() makes patch a no-op
--

commit 8e400c22f46c7fff9d91b8c8e837ab24dfa0f5cd
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Fix regex_match_end() for zero-length strings

This commit adds an early return in regex_match_end() when len <= r->len to
avoid underflowing len - 1 and accessing the input string when len is 0 or
smaller than the pattern.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a3..59a0d3ba72740 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1075,8 +1075,12 @@ static int regex_match_middle(char *str, struct regex *r, int len)
>  
>  static int regex_match_end(char *str, struct regex *r, int len)
>  {
> -	int strlen = len - 1;
> +	int strlen;
>  
> +	if (len <= r->len)
> +		return 0;
> +
> +	strlen = len - 1;
>  	if (strlen >= r->len &&
>  	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)
>  		return 1;

[Severity: Medium]
Does this change have any functional effect in regex_match_end() in
kernel/trace/trace_events_filter.c?

In regex_match_end(), len, strlen, and r->len (defined in struct regex in
kernel/trace/trace.h and initialized from strlen(search) >= 0 in
filter_build_regex()) are all signed int variables.

When len is 0, strlen = len - 1 evaluates to -1. Because both strlen and
r->len are signed ints and r->len >= 0, the existing check:

	if (strlen >= r->len &&
	    memcmp(str + strlen - r->len, r->pattern, r->len) == 0)

evaluates -1 >= r->len, which is false and short-circuits before calling
memcmp() or dereferencing str.

More generally, for signed integers len >= 0 and r->len >= 0, the new check
len <= r->len is equivalent to (len - 1) < r->len (i.e. strlen < r->len),
which is the exact negation of the existing strlen >= r->len check. With the
early return in place, strlen >= r->len is also unconditionally true and
redundant.

Since the pre-existing condition already prevented out-of-bounds access when
len == 0 or len <= r->len, is this patch (along with the Fixes and stable
tags) necessary, or was a different change intended here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179314820431.684037.4147763044424284198.stgit@devnote2?part=1