2010-01-14 02:52:43

by Li Zefan

[permalink] [raw]
Subject: [PATCH 0/7][RESEND] 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(-)


2010-01-14 02:53:05

by Li Zefan

[permalink] [raw]
Subject: [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

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/ftrace.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7968762..1e6640f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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;
}
--
1.6.3

2010-01-14 02:53:23

by Li Zefan

[permalink] [raw]
Subject: [PATCH 2/7] 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().

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 74563d7..6d7b8f5 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}
--
1.6.3

2010-01-14 02:53:42

by Li Zefan

[permalink] [raw]
Subject: [PATCH 3/7] tracing/filters: Fix MATCH_END_ONLY filter matching

'foo', but tracing filter incorrectly disallows strings
matching regex expression ".*foo.*foo".

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events_filter.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 6d7b8f5..c6a0f8d 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}
--
1.6.3

2010-01-14 02:53:55

by Li Zefan

[permalink] [raw]
Subject: [PATCH 4/7] lib: Introduce strnstr()

It differs strstr() in that it limits the length to be searched
in the first string.

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
include/linux/string.h | 5 ++++-
lib/string.c | 27 ++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 651839a..a716ee2 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -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_STRLEN
extern __kernel_size_t strlen(const char *);
diff --git a/lib/string.c b/lib/string.c
index 9f75b4e..a1cdcfc 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -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.
--
1.6.3

2010-01-14 02:54:12

by Li Zefan

[permalink] [raw]
Subject: [PATCH 5/7] 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().

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c6a0f8d..c43ebb3 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}
--
1.6.3

2010-01-14 02:54:28

by Li Zefan

[permalink] [raw]
Subject: [PATCH 6/7] tracing/filters: Fix MATCH_END_ONLY 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.

Signed-off-by: Li Zefan <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events_filter.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c43ebb3..14e83de 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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);
--
1.6.3

2010-01-14 02:54:42

by Li Zefan

[permalink] [raw]
Subject: [PATCH 7/7] 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.

Signed-off-by: Li Zefan <[email protected]>
---
kernel/trace/trace_events_filter.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 14e83de..4615f62 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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)
--
1.6.3

2010-01-18 14:54:11

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 4/7] lib: Introduce strnstr()

On Sat, 2010-01-16 at 12:12 +0100, Alex Riesen wrote:
> On Thu, Jan 14, 2010 at 03:53, Li Zefan <[email protected]> wrote:
> > @@ -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.

>
> > @@ -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;
>
> Are you sure you want to search _past_ the NUL-terminator
> of s1?
>
> > + 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

> > + if (!memcmp(s1, s2, l2))
> > + return (char *)s1;
> > + s1++;
> > + }
> > + return NULL;
> > +}

2010-01-18 15:56:09

by Alex Riesen

[permalink] [raw]
Subject: Re: [PATCH 4/7] lib: Introduce strnstr()

On Mon, Jan 18, 2010 at 15:53, Steven Rostedt <[email protected]> wrote:
> On Sat, 2010-01-16 at 12:12 +0100, Alex Riesen wrote:
>>
>> Are you sure you want to search _past_ the NUL-terminator
>> of s1?
>>
>> > +       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.

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).

2010-01-16 09:43:23

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] ftrace: Fix MATCH_END_ONLY function filter

Commit-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

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/ftrace.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7968762..1e6640f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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;
}

2010-01-16 09:43:42

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] tracing/filters: Fix MATCH_FRONT_ONLY filter matching

Commit-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().

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 50504cb..11c3973 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}

2010-01-16 09:43:56

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] tracing/filters: Fix MATCH_END_ONLY filter matching

Commit-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:

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_filter.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 11c3973..49e44dd 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}

2010-01-16 09:44:13

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] lib: Introduce strnstr()

Commit-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.

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
include/linux/string.h | 5 ++++-
lib/string.c | 27 ++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 651839a..a716ee2 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -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_STRLEN
extern __kernel_size_t strlen(const char *);
diff --git a/lib/string.c b/lib/string.c
index 9f75b4e..a1cdcfc 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -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.

2010-01-16 09:44:32

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] tracing/filters: Fix MATCH_MIDDLE_ONLY filter matching

Commit-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().

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 49e44dd..f364b08 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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;
}

2010-01-16 09:44:45

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] tracing/filters: Fix MATCH_FULL filter matching for PTR_STRING

Commit-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.

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_filter.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index f364b08..60c2a4e 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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);

2010-01-16 09:44:59

by Li Zefan

[permalink] [raw]
Subject: [tip:tracing/urgent] tracing/filters: Add comment for match callbacks

Commit-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.

Signed-off-by: Li Zefan <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/trace/trace_events_filter.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 60c2a4e..e42af9a 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -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)

2010-01-16 11:13:11

by Alex Riesen

[permalink] [raw]
Subject: Re: [PATCH 4/7] lib: Introduce strnstr()

On Thu, Jan 14, 2010 at 03:53, Li Zefan <[email protected]> wrote:
> @@ -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?

> @@ -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;

Are you sure you want to search _past_ the NUL-terminator
of s1?

> +       l2 = strlen(s2);
> +       if (!l2)
> +               return (char *)s1;
> +       while (l1 >= l2) {
> +               l1--;
> +               if (!memcmp(s1, s2, l2))
> +                       return (char *)s1;
> +               s1++;
> +       }
> +       return NULL;
> +}