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) [off-list ref]
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.
quoted hunk ↗ jump to hunk
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