tracing: Fix bugs in string filters.
Subsystems (4)
cover tracing: Fix bugs in string filters.
This patchset fixes some bugs in string filters. Those bugs affect both ftrace and perf-trace.
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:
# ./perf record -R -f -a -e lock:lock_acquire \ --filter 'name ~rcu_*' sleep 1 # ./perf trace (no output)
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 Zefan'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
@@ -72,7 +72,10 @@ static inline __must_check char *strstrip(char *str) }#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
@@ -667,7 +667,7 @@ EXPORT_SYMBOL(memscan);*/ char *strstr (const char *s1,const char *s2) { -int l1, l2; +size_t l1, l2; l2 =strlen (s2);if (!l2)
@@ -684,6 +684,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
[tip:tracing/urgent] ftrace: Fix MATCH_END_ONLY function filter
In reply to Li ZefanCommit-ID: 751e9983ee276cb150e8812b1d995f6035a63878 Gitweb: http://git.kernel.org/tip/751e9983ee276cb150e8812b1d995f6035a63878 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:53:02 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:03 -0500
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
LKML-Reference: <[email protected]>
--- 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 ; }
[tip:tracing/urgent] tracing/filters: Fix MATCH_FRONT_ONLY filter matching
In reply to Li ZefanCommit-ID: 285caad415f459f336247932b4db95a571357a02 Gitweb: http://git.kernel.org/tip/285caad415f459f336247932b4db95a571357a02 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:53:21 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:05 -0500
tracing/filters: Fix MATCH_FRONT_ONLY filter matching
MATCH_FRONT_ONLY actually is a full matching:
# ./perf record -R -f -a -e lock:lock_acquire \ --filter 'name ~rcu_*' sleep 1 # ./perf trace (no output)
We should pass the length of the pattern string to strncmp().
LKML-Reference: <[email protected]>
--- 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 ; }
[tip:tracing/urgent] tracing/filters: Fix MATCH_END_ONLY filter matching
In reply to Li ZefanCommit-ID: a3291c14ecf0a995e30d993b7f2cae031de98727 Gitweb: http://git.kernel.org/tip/a3291c14ecf0a995e30d993b7f2cae031de98727 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:53:41 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:07 -0500
tracing/filters: Fix MATCH_END_ONLY filter matching
For '*foo' pattern, we should allow any string ending with 'foo', but event filtering incorrectly disallows strings like bar_foo_foo:
LKML-Reference: <[email protected]>
--- 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 ; }
[tip:tracing/urgent] lib: Introduce strnstr()
In reply to Li ZefanCommit-ID: d5f1fb53353edc38da326445267c1df0c9676df2 Gitweb: http://git.kernel.org/tip/d5f1fb53353edc38da326445267c1df0c9676df2 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:53:55 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:09 -0500
lib: Introduce strnstr()
It differs strstr() in that it limits the length to be searched in the first string.
LKML-Reference: <[email protected]>
--- include/linux/string.h | 5 ++++- lib/string.c | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-)
include/linux/string.h +4 -1
@@ -72,7 +72,10 @@ static inline __must_check char *strstrip(char *str) }#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
@@ -667,7 +667,7 @@ EXPORT_SYMBOL(memscan);*/ char *strstr (const char *s1,const char *s2) { -int l1, l2; +size_t l1, l2; l2 =strlen (s2);if (!l2)
@@ -684,6 +684,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.
[tip:tracing/urgent] tracing/filters: Fix MATCH_MIDDLE_ONLY filter matching
In reply to Li ZefanCommit-ID: b2af211f284eb1bef19fbb85fc8ef551bb1e7460 Gitweb: http://git.kernel.org/tip/b2af211f284eb1bef19fbb85fc8ef551bb1e7460 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:54:11 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:11 -0500
tracing/filters: Fix MATCH_MIDDLE_ONLY filter matching
The @str might not be NULL-terminated if it's of type DYN_STRING or STATIC_STRING, so we should use strnstr() instead of strstr().
LKML-Reference: <[email protected]>
--- 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 ; }
[tip:tracing/urgent] tracing/filters: Fix MATCH_FULL filter matching for PTR_STRING
In reply to Li ZefanCommit-ID: 16da27a8bc7a0d050686d1b2e9efb53fab9ed226 Gitweb: http://git.kernel.org/tip/16da27a8bc7a0d050686d1b2e9efb53fab9ed226 Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:54:27 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:12 -0500
tracing/filters: Fix MATCH_FULL filter matching for PTR_STRING
MATCH_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.
LKML-Reference: <[email protected]>
--- 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);
[tip:tracing/urgent] tracing/filters: Add comment for match callbacks
In reply to Li ZefanCommit-ID: d1303dd1d6b220cab375f24fa91a5640e54e169e Gitweb: http://git.kernel.org/tip/d1303dd1d6b220cab375f24fa91a5640e54e169e Author: Li Zefan <[email protected]> AuthorDate: Thu, 14 Jan 2010 10:54:40 +0800 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 14 Jan 2010 22:38:14 -0500
tracing/filters: Add comment for match callbacks
We 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.
LKML-Reference: <[email protected]>
--- 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 )
Re: [PATCH 4/7] lib: Introduce strnstr()
In reply to Li Zefanlib/string.c EXPORT_SYMBOL(memscan); · line 671
Quoted 7 lines · Li Zefan
lib/string.c +1
-1@@ -667,7 +667,7 @@ EXPORT_SYMBOL(memscan);*/ char *strstr(const char *s1, const char *s2) { - int l1, l2; + size_t l1, l2;
This chunk is not related, is it?
Quoted 4 lines
lib/string.c +0
-0@@ -684,6 +684,31 @@ char *strstr(const char *s1, const char *s2) EXPORT_SYMBOL(strstr); #endif
lib/string.c char *strstr(const char *s1, const char *s2) · line 696
Quoted 10 lines
lib/string.c +10
-0+#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;
Are you sure you want to search _past_ the NUL-terminator of s1?
Quoted 11 lines
lib/string.c +11
-0+ l2 = strlen(s2); + if (!l2) + return (char *)s1; + while (l1 >= l2) { + l1--; + if (!memcmp(s1, s2, l2)) + return (char *)s1; + s1++; + } + return NULL; +}
Steven Rostedt
maintainerRe: [PATCH 4/7] lib: Introduce strnstr()
Quoted 10 lines · Alex Riesen
lib/string.c EXPORT_SYMBOL(memscan); · line 671
Quoted 7 lines · Li Zefan
lib/string.c +1
-1@@ -667,7 +667,7 @@ EXPORT_SYMBOL(memscan);*/ char *strstr (const char *s1,const char *s2) { -int l1, l2; +size_t l1, l2;This chunk is not related, is it?
Actually it is related. Making both strstr and strnstr use the correct variable and be consistent. This patch introduces strnstr and in doing so also makes strstr consistent (and correct) with strnstr.
Quoted 5 lines
Quoted 5 lines
lib/string.c +0
-0@@ -684,6 +684,31 @@ char *strstr(const char *s1, const char *s2)EXPORT_SYMBOL (strstr);#endif
Quoted 19 lines
lib/string.c char *strstr(const char *s1, const char *s2) · line 696
Quoted 10 lines
lib/string.c +10
-0+#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;Are you sure you want to search _past_ the NUL-terminator of s1?
Quoted 5 lines
lib/string.c +5
-0+ l2 =strlen (s2); +if (!l2) +return (char *)s1; +while (l1 >= l2) { + l1--;
The first check is len-1, I don't see it searching past the NUL-terminator. The loop will stop when l1 == l2 (the size of s2) and s1 pointing near the end of the string.
-- Steve
Quoted 6 lines
Quoted 6 lines
lib/string.c +6
-0+if (!memcmp (s1, s2, l2)) +return (char *)s1; + s1++; + } +return NULL ; +}
Re: [PATCH 4/7] lib: Introduce strnstr()
Quoted 11 lines · Steven Rostedt
Quoted 9 lines · Alex Riesen
Are you sure you want to search _past_ the NUL-terminator of s1?
Quoted 5 lines
+ l2 = strlen(s2);
+ if (!l2)
+ return (char *)s1;
+ while (l1 >= l2) {
+ l1--;
Quoted 3 lines
The first check is len-1, I don't see it searching past the NUL-terminator. The loop will stop when l1 == l2 (the size of s2) and s1 pointing near the end of the string.
I thought that len means "minimum of strlen(s1) and len".
Li already explained that was not intended, even if s2 cannot be made to contain a \0, as s1 can by specifying len greater than strlen(s1).