[PATCH 2/6] tracing: Fix parsing of globs with a wildcard at the beginning

2018-02-09 Thread Steven Rostedt 
From: "Steven Rostedt (VMware)" 

Al Viro reported:

For substring - sure, but what about something like "*a*b" and "a*b"?
AFAICS, filter_parse_regex() ends up with identical results in both
cases - MATCH_GLOB and *search = "a*b".  And no way for the caller
to tell one from another.

Testing this with the following:

 # cd /sys/kernel/tracing
 # echo '*raw*lock' > set_ftrace_filter
 bash: echo: write error: Invalid argument

With this patch:

 # echo '*raw*lock' > set_ftrace_filter
 # cat set_ftrace_filter
_raw_read_trylock
_raw_write_trylock
_raw_read_unlock
_raw_spin_unlock
_raw_write_unlock
_raw_spin_trylock
_raw_spin_lock
_raw_write_lock
_raw_read_lock

Al recommended not setting the search buffer to skip the first '*' unless we
know we are not using MATCH_GLOB. This implements his suggested logic.

Link: http://lkml.kernel.org/r/20180127170748.gf13...@zeniv.linux.org.uk

Cc: sta...@vger.kernel.org
Fixes: 60f1d5e3bac44 ("ftrace: Support full glob matching")
Reviewed-by: Masami Hiramatsu 
Reported-by: Al Viro 
Suggsted-by: Al Viro 
Signed-off-by: Steven Rostedt (VMware) 
---
 kernel/trace/trace_events_filter.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c 
b/kernel/trace/trace_events_filter.c
index 61e7f0678d33..a764aec3c9a1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -400,7 +400,6 @@ enum regex_type filter_parse_regex(char *buff, int len, 
char **search, int *not)
for (i = 0; i < len; i++) {
if (buff[i] == '*') {
if (!i) {
-   *search = buff + 1;
type = MATCH_END_ONLY;
} else if (i == len - 1) {
if (type == MATCH_END_ONLY)
@@ -410,14 +409,14 @@ enum regex_type filter_parse_regex(char *buff, int len, 
char **search, int *not)
buff[i] = 0;
break;
} else {/* pattern continues, use full glob */
-   type = MATCH_GLOB;
-   break;
+   return MATCH_GLOB;
}
} else if (strchr("[?\\", buff[i])) {
-   type = MATCH_GLOB;
-   break;
+   return MATCH_GLOB;
}
}
+   if (buff[0] == '*')
+   *search = buff + 1;
 
return type;
 }
-- 
2.15.1

Re: [RFC][PATCH 2/6] tracing: Fix parsing of globs with a wildcard at the beginning

2018-02-08 Thread Masami Hiramatsu 
On Tue, 06 Feb 2018 17:43:40 -0500
Steven Rostedt  wrote:

> From: "Steven Rostedt (VMware)" 
> 
> Al Viro reported:
> 
> For substring - sure, but what about something like "*a*b" and "a*b"?
> AFAICS, filter_parse_regex() ends up with identical results in both
> cases - MATCH_GLOB and *search = "a*b".  And no way for the caller
> to tell one from another.
> 
> Testing this with the following:
> 
>  # cd /sys/kernel/tracing
>  # echo '*raw*lock' > set_ftrace_filter
>  bash: echo: write error: Invalid argument
> 
> With this patch:
> 
>  # echo '*raw*lock' > set_ftrace_filter
>  # cat set_ftrace_filter
> _raw_read_trylock
> _raw_write_trylock
> _raw_read_unlock
> _raw_spin_unlock
> _raw_write_unlock
> _raw_spin_trylock
> _raw_spin_lock
> _raw_write_lock
> _raw_read_lock
> 
> Al recommended not setting the search buffer to skip the first '*' unless we
> know we are not using MATCH_GLOB. This implements his suggested logic.
> 
> Link: http://lkml.kernel.org/r/20180127170748.gf13...@zeniv.linux.org.uk
> 
> Cc: sta...@vger.kernel.org
> Cc: Masami Hiramatsu 
> Fixes: 60f1d5e3bac44 ("ftrace: Support full glob matching")
> Reported-by: Al Viro 
> Suggsted-by: Al Viro 
> Signed-off-by: Steven Rostedt (VMware) 

This looks good to me.

Reviewed-by: Masami Hiramatsu 

Thank you!

> ---
>  kernel/trace/trace_events_filter.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c 
> b/kernel/trace/trace_events_filter.c
> index 61e7f0678d33..a764aec3c9a1 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -400,7 +400,6 @@ enum regex_type filter_parse_regex(char *buff, int len, 
> char **search, int *not)
>   for (i = 0; i < len; i++) {
>   if (buff[i] == '*') {
>   if (!i) {
> - *search = buff + 1;
>   type = MATCH_END_ONLY;
>   } else if (i == len - 1) {
>   if (type == MATCH_END_ONLY)
> @@ -410,14 +409,14 @@ enum regex_type filter_parse_regex(char *buff, int len, 
> char **search, int *not)
>   buff[i] = 0;
>   break;
>   } else {/* pattern continues, use full glob */
> - type = MATCH_GLOB;
> - break;
> + return MATCH_GLOB;
>   }
>   } else if (strchr("[?\\", buff[i])) {
> - type = MATCH_GLOB;
> - break;
> + return MATCH_GLOB;
>   }
>   }
> + if (buff[0] == '*')
> + *search = buff + 1;
>  
>   return type;
>  }
> -- 
> 2.15.1
> 
> 


-- 
Masami Hiramatsu

[RFC][PATCH 2/6] tracing: Fix parsing of globs with a wildcard at the beginning

2018-02-06 Thread Steven Rostedt 
From: "Steven Rostedt (VMware)" 

Al Viro reported:

For substring - sure, but what about something like "*a*b" and "a*b"?
AFAICS, filter_parse_regex() ends up with identical results in both
cases - MATCH_GLOB and *search = "a*b".  And no way for the caller
to tell one from another.

Testing this with the following:

 # cd /sys/kernel/tracing
 # echo '*raw*lock' > set_ftrace_filter
 bash: echo: write error: Invalid argument

With this patch:

 # echo '*raw*lock' > set_ftrace_filter
 # cat set_ftrace_filter
_raw_read_trylock
_raw_write_trylock
_raw_read_unlock
_raw_spin_unlock
_raw_write_unlock
_raw_spin_trylock
_raw_spin_lock
_raw_write_lock
_raw_read_lock

Al recommended not setting the search buffer to skip the first '*' unless we
know we are not using MATCH_GLOB. This implements his suggested logic.

Link: http://lkml.kernel.org/r/20180127170748.gf13...@zeniv.linux.org.uk

Cc: sta...@vger.kernel.org
Cc: Masami Hiramatsu 
Fixes: 60f1d5e3bac44 ("ftrace: Support full glob matching")
Reported-by: Al Viro 
Suggsted-by: Al Viro 
Signed-off-by: Steven Rostedt (VMware) 
---
 kernel/trace/trace_events_filter.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c 
b/kernel/trace/trace_events_filter.c
index 61e7f0678d33..a764aec3c9a1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -400,7 +400,6 @@ enum regex_type filter_parse_regex(char *buff, int len, 
char **search, int *not)
for (i = 0; i < len; i++) {
if (buff[i] == '*') {
if (!i) {
-   *search = buff + 1;
type = MATCH_END_ONLY;
} else if (i == len - 1) {
if (type == MATCH_END_ONLY)
@@ -410,14 +409,14 @@ enum regex_type filter_parse_regex(char *buff, int len, 
char **search, int *not)
buff[i] = 0;
break;
} else {/* pattern continues, use full glob */
-   type = MATCH_GLOB;
-   break;
+   return MATCH_GLOB;
}
} else if (strchr("[?\\", buff[i])) {
-   type = MATCH_GLOB;
-   break;
+   return MATCH_GLOB;
}
}
+   if (buff[0] == '*')
+   *search = buff + 1;
 
return type;
 }
-- 
2.15.1