Re: [PATCH 01/10] tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing/filters: out-of-bounds read in regex_match_end() when len is 0
--

commit 1023475f1fd5bcb5e8b46a2be2fb9fe7ea16d624
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings

This commit updates regex_match_end() to compute string length via strnlen()
when len is non-zero and strlen() when len is zero, fixing suffix filter
matching on fixed-size and dynamic string fields.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a3..18ce3ec00d34f 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1075,10 +1075,10 @@ 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 slen = len ? strnlen(str, len) : strlen(str);

[Severity: High]
Can calling strlen(str) when len == 0 cause an out-of-bounds read past the
end of the trace event record?

While filter_pred_string() passes pred->regex->field_len == 0 for
NUL-terminated dynamic strings, regex_match_end() is also called by
filter_pred_strloc() and filter_pred_strrelloc(), where len is the runtime
byte length extracted from the locator:

kernel/trace/trace_events_filter.c:filter_pred_strloc() {
	u32 str_item = *(u32 *)(event + pred->offset);
	int str_loc = str_item & 0xffff;
	int str_len = str_item >> 16;
	char *addr = (char *)(event + str_loc);
	int cmp, match;

	cmp = pred->regex->match(addr, pred->regex, str_len);
}

When a dynamic char array has a runtime length of 0, str_len is 0 and addr
points to the end of the event or uninitialized trailing memory without a
NUL terminator.

For example, in fs/xfs/xfs_trace.h, xfs_da_class defines a dynamic char array
that isclassified as FILTER_DYN_STRING, and when xfs_dir_init() ->
xfs_dir2_sf_create() runs with args->namelen == 0, 0 bytes are reserved and
nothing is written:

	__dynamic_array(char, name, args->namelen)
...
	TP_fast_assign(
		__entry->dev = VFS_I(args->dp)->i_sb->s_dev;
		__entry->ino = I_INO(args->dp);
		if (args->namelen)
			memcpy(__get_str(name), args->name, args->namelen);

Similarly, in kernel/trace/trace_events_user.c, user_event_validate() allows
a user_events string locator with size == 0 and pos == end as long as the
preceding byte *(char *)(end - 1) is '\0':

		pos += size;

		if (unlikely(pos > end))
			return -EFAULT;

		if (likely(validator->flags & VALIDATOR_ENSURE_NULL))
			if (unlikely(*(char *)(pos - 1) != '\0'))
				return -EFAULT;

In both cases, evaluating a MATCH_END_ONLY filter calls regex_match_end()
with len == 0 and str pointing at or past the end of the event buffer, where
strlen(str) and memcmp() will read out of bounds.

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