tracing: Fix bugs in string filters.
Subsystems (4)
cover tracing: Fix bugs in string filters.
This patchset fixes some bugs in string filters.
A new string helper strnstr() is introduced, to be used in middle matching callback.
Also I add some comments to mainly explain why we should use length-limited version of strstr() and strcmp().
--- include/linux/string.h | 5 ++++- kernel/trace/ftrace.c | 6 +++--- kernel/trace/trace_events_filter.c | 29 ++++++++++++++++++++--------- lib/string.c | 27 ++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 14 deletions(-)
PATCH 1/7 ftrace: Fix MATCH_END_ONLY function filter
For '*foo' pattern, we should allow any string ending with 'foo', but ftrace filter incorrectly disallows strings like bar_foo_foo:
# echo '*io' > set_ftrace_filter # cat set_ftrace_filter | grep 'req_bio_endio' # cat available_filter_functions | grep 'req_bio_endio' req_bio_endio
--- kernel/trace/ftrace.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
kernel/trace/ftrace.c +3 -3
@@ -1690,7 +1690,7 @@ ftrace_regex_lseek(struct file *file, loff_t offset, int origin)static int ftrace_match (char *str,char *regex,int len,int type) {int matched =0 ; -char *ptr; +int slen;switch (type) {case MATCH_FULL :
@@ -1706,8 +1706,8 @@ static int ftrace_match(char *str, char *regex, int len, int type) matched =1 ;break ;case MATCH_END_ONLY : - ptr =strstr (str, regex); -if (ptr && (ptr[len] ==0 )) + slen =strlen (str); +if (slen >= len &&memcmp (str + slen - len, regex, len) ==0 ) matched =1 ;break ; }
Signature · 1 line
1.6.3
PATCH 2/7 tracing/filters: Fix MATCH_FRONT_ONLY filter matching
In reply to Li ZefanMATCH_FRONT_ONLY actually is a full matching.
We should pass the length of the pattern string to strncmp().
--- kernel/trace/trace_events_filter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/trace/trace_events_filter.c +1 -1
@@ -261,7 +261,7 @@ static int regex_match_full(char *str, struct regex *r, int len)static int regex_match_front (char *str,struct regex *r,int len) { -if (strncmp (str, r->pattern, len) ==0 ) +if (strncmp (str, r->pattern, r->len) ==0 )return 1 ;return 0 ; }
Signature · 1 line
1.6.3
PATCH 3/7 tracing/filters: Fix MATCH_END_ONLY filter matching
In reply to Li ZefanFor '*foo' pattern, we should allow any string ending with 'foo', but tracing filter incorrectly disallows strings matching regex expression ".*foo.*foo".
--- kernel/trace/trace_events_filter.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
kernel/trace/trace_events_filter.c +3 -2
@@ -275,9 +275,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) { -char *ptr =strstr (str, r->pattern); +int strlen = len -1 ; -if (ptr && (ptr[r->len] ==0 )) +if (strlen >= r->len && +memcmp (str + strlen - r->len, r->pattern, r->len) ==0 )return 1 ;return 0 ; }
Signature · 1 line
1.6.3
PATCH 4/7 lib: Introduce strnstr()
In reply to Li ZefanIt differs strstr() in that it limits the length to be searched in the first string.
--- include/linux/string.h | 5 ++++- lib/string.c | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-)
include/linux/string.h +4 -1
@@ -64,7 +64,10 @@ extern char * strrchr(const char *,int);#endif extern char *__must_check strstrip (char *);#ifndef __HAVE_ARCH_STRSTR -extern char *strstr (const char *,const char *); +extern char *strstr (const char *,const char *); +#endif +#ifndef __HAVE_ARCH_STRNSTR +extern char *strnstr (const char *,const char *,size_t );#endif #ifndef __HAVE_ARCH_STRLENextern __kernel_size_t strlen (const char *);
lib/string.c +26 -1
@@ -656,7 +656,7 @@ EXPORT_SYMBOL(memscan);*/ char *strstr (const char *s1,const char *s2) { -int l1, l2; +size_t l1, l2; l2 =strlen (s2);if (!l2)
@@ -673,6 +673,31 @@ char *strstr(const char *s1, const char *s2)EXPORT_SYMBOL (strstr);#endif +#ifndef __HAVE_ARCH_STRNSTR +/** +* strnstr - Find the first substring in a length-limited string +* @s1: The string to be searched +* @s2: The string to search for +* @len: the maximum number of characters to search +*/ +char *strnstr (const char *s1,const char *s2,size_t len) +{ +size_t l1 = len, l2; + + l2 =strlen (s2); +if (!l2) +return (char *)s1; +while (l1 >= l2) { + l1--; +if (!memcmp (s1, s2, l2)) +return (char *)s1; + s1++; + } +return NULL ; +} +EXPORT_SYMBOL (strnstr); +#endif +#ifndef __HAVE_ARCH_MEMCHR/** * memchr - Find a character in an area of memory.
Signature · 1 line
1.6.3
PATCH 5/7 tracing/filters: Fix MATCH_MIDDLE_ONLY filter matching
In reply to Li ZefanThe @str might not be NULL-terminated if it's of type DYN_STRING or STATIC_STRING, so we should use strnstr() instead of strstr().
--- kernel/trace/trace_events_filter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/trace/trace_events_filter.c +1 -1
@@ -268,7 +268,7 @@ static int regex_match_front(char *str, struct regex *r, int len)static int regex_match_middle (char *str,struct regex *r,int len) { -if (strstr (str, r->pattern)) +if (strnstr (str, r->pattern, len))return 1 ;return 0 ; }
Signature · 1 line
1.6.3
PATCH 6/7 tracing/filters: Fix MATCH_END_ONLY filter matching for PTR_STRING
In reply to Li ZefanMATCH_FULL matching for PTR_STRING is not working correctly:
# echo 'func == vt' > events/bkl/lock_kernel/filter
# echo 1 > events/bkl/lock_kernel/enable
...
# cat trace
Xorg-1484 [000] 1973.392586: lock_kernel: ... func=vt_ioctl()
gpm-1402 [001] 1974.027740: lock_kernel: ... func=vt_ioctl()We should pass to regex.match(..., len) the length (including '\0') of the source string instead of the length of the pattern string.
--- kernel/trace/trace_events_filter.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
kernel/trace/trace_events_filter.c +3 -4
@@ -211,8 +211,9 @@ static int filter_pred_pchar(struct filter_pred *pred, void *event, {char **addr = (char **)(event + pred->offset);int cmp, match; +int len =strlen (*addr) +1 ;/* including tailing '\0' */ - cmp = pred->regex.match (*addr, &pred->regex, pred->regex.field_len); + cmp = pred->regex.match (*addr, &pred->regex, len); match = cmp ^ pred->not;
@@ -782,10 +783,8 @@ static int filter_add_pred(struct filter_parse_state *ps, pred->regex.field_len = field->size; }else if (field->filter_type ==FILTER_DYN_STRING ) fn = filter_pred_strloc; -else { +else fn = filter_pred_pchar; - pred->regex.field_len =strlen (pred->regex.pattern); - } }else {if (field->is_signed) ret =strict_strtoll (pred->regex.pattern,0 , &val);
Signature · 1 line
1.6.3
PATCH 7/7 tracing/filters: Add comment for match callbacks
In reply to Li ZefanWe should be clear on 2 things:
- the length parameter of a match callback includes tailing '\0'.
- the string to be searched might not be NULL-terminated.
--- kernel/trace/trace_events_filter.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
kernel/trace/trace_events_filter.c +12 -1
@@ -252,7 +252,18 @@ static int filter_pred_none(struct filter_pred *pred, void *event,return 0 ; } -/* Basic regex callbacks */ +/* +* regex_match_foo - Basic regex callbacks +* +* @str: the string to be searched +* @r: the regex structure containing the pattern string +* @len: the length of the string to be searched (including '\0') +* +* Note: +* - @str might not be NULL-terminated if it's of type DYN_STRING +* or STATIC_STRING +*/ +static int regex_match_full (char *str,struct regex *r,int len) {if (strncmp (str, r->pattern, len) ==0 )
Signature · 1 line
1.6.3
Quoted 8 lines · Li Zefan
This patchset fixes some bugs in string filters.
A new string helper
strnstr()is introduced, to be used in middle matching callback.Also I add some comments to mainly explain why we should use length-limited version of
strstr()andstrcmp().
Quoted 6 lines
--- include/linux/string.h | 5 ++++- kernel/trace/ftrace.c | 6 +++--- kernel/trace/trace_events_filter.c | 29 ++++++++++++++++++++--------- lib/string.c | 27 ++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 14 deletions(-)
Looks pretty good.
Thanks!
Quoted 17 lines · Frederic Weisbecker
Quoted 8 lines · Li Zefan
This patchset fixes some bugs in string filters.
A new string helper
strnstr()is introduced, to be used in middle matching callback.Also I add some comments to mainly explain why we should use length-limited version of
strstr()andstrcmp().Quoted 6 lines
--- include/linux/string.h | 5 ++++- kernel/trace/ftrace.c | 6 +++--- kernel/trace/trace_events_filter.c | 29 ++++++++++++++++++++--------- lib/string.c | 27 ++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 14 deletions(-)
Quoted 4 lines
Looks pretty good.
Hi Steven, could you pick up those patches?