[RFC][PATCHES] constifying ftrace globs

[RFC][PATCHES] constifying ftrace globs

[Al Viro]

	We don't really need to NUL-terminate the substring we are
matching against; all it takes is introducing memmem(3) and using it
instead of str[n]str().

	It's not that much work - see vfs.git#ftrace_glob.  The reason
I went there is rather amusing; it all started with making do_execve()
take arrays of const strings for argv and envp.  After all, we never
change them *and* we often enough pass arrays of string literals that
way.  It exploded into a series of 75 commits, with the final ripple
being argv_split() and argv_free().  OK, turns out that ftrace is
using those as well, fortunately it's done to get arrays of regexes,
so it should be trivial to constify as well, right?  Imagine the amount of
swearing when I noticed that it *does* modify the resulting strings...

	This series deals with that problem, providing the missing prereq for
the patchbomb from hell...

	Could you review #ftrace_glob?  It's not large - seven commits,
boiling down to
 include/linux/ftrace.h             |  8 ++---
 include/linux/string.h             |  1 +
 kernel/trace/ftrace.c              | 68 ++++++++++++++++++--------------------
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c | 26 +++++++++------
 lib/string.c                       | 34 ++++++++++++-------
 6 files changed, 75 insertions(+), 64 deletions(-)

Patches in followups.

[RFC][PATCH 1/7] trace: fix the glob match in __unregister_ftrace_function_probe()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

For patterns starting with '*' we need to match against 'search', not
'glob'.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 224e768..9f90a4f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3802,7 +3802,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 			if (glob) {
 				kallsyms_lookup(entry->ip, NULL, NULL,
 						NULL, str);
-				if (!ftrace_match(str, glob, len, type))
+				if (!ftrace_match(str, search, len, type))
 					continue;
 			}
 
-- 
2.1.4

[RFC][PATCH 2/7] implement memmem()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/string.h |  1 +
 lib/string.c           | 34 ++++++++++++++++++++++------------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 2e22a2e..87f9fba 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -75,6 +75,7 @@ extern char * strstr(const char *, const char *);
 #endif
 #ifndef __HAVE_ARCH_STRNSTR
 extern char * strnstr(const char *, const char *, size_t);
+extern void * memmem(const void *, size_t, const void *, 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 1006330..2035dbe 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -740,27 +740,37 @@ EXPORT_SYMBOL(strstr);
 #endif
 
 #ifndef __HAVE_ARCH_STRNSTR
+
 /**
- * strnstr - Find the first substring in a length-limited string
+ * memmem - Find the first length-limited substring in a length-limited string
  * @s1: The string to be searched
+ * @len1: the maximum number of characters to search
  * @s2: The string to search for
- * @len: the maximum number of characters to search
+ * @len2: the length of the string being searched
  */
-char *strnstr(const char *s1, const char *s2, size_t len)
+void *memmem(const void *s1, size_t len1, const void *s2, size_t len2)
 {
-	size_t l2;
-
-	l2 = strlen(s2);
-	if (!l2)
-		return (char *)s1;
-	while (len >= l2) {
-		len--;
-		if (!memcmp(s1, s2, l2))
-			return (char *)s1;
+	if (!len2)
+		return (void *)s1;
+	while (len1 >= len2) {
+		len1--;
+		if (!memcmp(s1, s2, len2))
+			return (void *)s1;
 		s1++;
 	}
 	return NULL;
 }
+EXPORT_SYMBOL(memmem);
+/**
+ * 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)
+{
+	return memmem(s1, len, s2, strlen(s2));
+}
 EXPORT_SYMBOL(strnstr);
 #endif
 
-- 
2.1.4

[RFC][PATCH 3/7] trace_events_filter.c: switch to memcmp() and memmem() for matching

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

we know the lengths in advance...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/trace_events_filter.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ced69da..6c4a96b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -271,21 +271,25 @@ static int filter_pred_none(struct filter_pred *pred, void *event)
 
 static int regex_match_full(char *str, struct regex *r, int len)
 {
-	if (strncmp(str, r->pattern, len) == 0)
+	if (len - 1 != r->len)
+		return 0;
+	if (memcmp(str, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }
 
 static int regex_match_front(char *str, struct regex *r, int len)
 {
-	if (strncmp(str, r->pattern, r->len) == 0)
+	if (len - 1 < r->len)
+		return 0;
+	if (memcmp(str, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }
 
 static int regex_match_middle(char *str, struct regex *r, int len)
 {
-	if (strnstr(str, r->pattern, len))
+	if (memmem(str, len, r->pattern, r->len))
 		return 1;
 	return 0;
 }
-- 
2.1.4

[RFC][PATCH 4/7] ftrace: switch matching to memcmp() and memmem()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

now we don't need the glob substring being matched to get
NUL-terminated by filter_parse_regex()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9f90a4f..6cb104a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3365,24 +3365,26 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 static int ftrace_match(char *str, char *regex, int len, int type)
 {
 	int matched = 0;
-	int slen;
+	int slen = strlen(str);
+
+	if (slen < len)
+		return 0;
 
 	switch (type) {
 	case MATCH_FULL:
-		if (strcmp(str, regex) == 0)
+		if (slen == len && memcmp(str, regex, len) == 0)
 			matched = 1;
 		break;
 	case MATCH_FRONT_ONLY:
-		if (strncmp(str, regex, len) == 0)
+		if (memcmp(str, regex, len) == 0)
 			matched = 1;
 		break;
 	case MATCH_MIDDLE_ONLY:
-		if (strstr(str, regex))
+		if (memmem(str, slen, regex, len))
 			matched = 1;
 		break;
 	case MATCH_END_ONLY:
-		slen = strlen(str);
-		if (slen >= len && memcmp(str + slen - len, regex, len) == 0)
+		if (memcmp(str + slen - len, regex, len) == 0)
 			matched = 1;
 		break;
 	}
-- 
2.1.4

[RFC][PATCH 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

... by passing len by address and using it to report the length of
substring in question.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c              | 24 +++++++++---------------
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c | 10 +++++-----
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6cb104a..e082681 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3441,7 +3441,6 @@ static int
 match_records(struct ftrace_hash *hash, char *buff,
 	      int len, char *mod, int not)
 {
-	unsigned search_len = 0;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	int type = MATCH_FULL;
@@ -3449,10 +3448,8 @@ match_records(struct ftrace_hash *hash, char *buff,
 	int found = 0;
 	int ret;
 
-	if (len) {
-		type = filter_parse_regex(buff, len, &search, &not);
-		search_len = strlen(search);
-	}
+	if (len)
+		type = filter_parse_regex(buff, &len, &search, &not);
 
 	mutex_lock(&ftrace_lock);
 
@@ -3460,7 +3457,7 @@ match_records(struct ftrace_hash *hash, char *buff,
 		goto out_unlock;
 
 	do_for_each_ftrace_rec(pg, rec) {
-		if (ftrace_match_record(rec, mod, search, search_len, type)) {
+		if (ftrace_match_record(rec, mod, search, len, type)) {
 			ret = enter_record(hash, rec, not);
 			if (ret < 0) {
 				found = ret;
@@ -3648,14 +3645,13 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	struct ftrace_hash *hash;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
-	int type, len, not;
+	int type, len = strlen(glob), not;
 	unsigned long key;
 	int count = 0;
 	char *search;
 	int ret;
 
-	type = filter_parse_regex(glob, strlen(glob), &search, &not);
-	len = strlen(search);
+	type = filter_parse_regex(glob, &len, &search, &not);
 
 	/* we do not support '!' for function probes */
 	if (WARN_ON(not))
@@ -3770,9 +3766,9 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 		glob = NULL;
 	else if (glob) {
 		int not;
+		len = strlen(glob);
 
-		type = filter_parse_regex(glob, strlen(glob), &search, &not);
-		len = strlen(search);
+		type = filter_parse_regex(glob, &len, &search, &not);
 
 		/* we do not support '!' for function probes */
 		if (WARN_ON(not))
@@ -4551,7 +4547,7 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 {
 	struct dyn_ftrace *rec;
 	struct ftrace_page *pg;
-	int search_len;
+	int search_len = strlen(buffer);
 	int fail = 1;
 	int type, not;
 	char *search;
@@ -4559,12 +4555,10 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 	int i;
 
 	/* decode regex */
-	type = filter_parse_regex(buffer, strlen(buffer), &search, &not);
+	type = filter_parse_regex(buffer, &search_len, &search, &not);
 	if (!not && *idx >= size)
 		return -EBUSY;
 
-	search_len = strlen(search);
-
 	mutex_lock(&ftrace_lock);
 
 	if (unlikely(ftrace_disabled)) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8de48ba..7483205 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1051,7 +1051,7 @@ struct filter_pred {
 };
 
 extern enum regex_type
-filter_parse_regex(char *buff, int len, char **search, int *not);
+filter_parse_regex(char *buff, int *len, char **search, int *not);
 extern void print_event_filter(struct ftrace_event_file *file,
 			       struct trace_seq *s);
 extern int apply_event_filter(struct ftrace_event_file *file,
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 6c4a96b..6a659e1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -321,7 +321,7 @@ static int regex_match_end(char *str, struct regex *r, int len)
  *  not returns 1 if buff started with a '!'
  *     0 otherwise.
  */
-enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
+enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
 {
 	int type = MATCH_FULL;
 	int i;
@@ -329,13 +329,13 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
 	if (buff[0] == '!') {
 		*not = 1;
 		buff++;
-		len--;
+		(*len)--;
 	} else
 		*not = 0;
 
 	*search = buff;
 
-	for (i = 0; i < len; i++) {
+	for (i = 0; i < *len; i++) {
 		if (buff[i] == '*') {
 			if (!i) {
 				*search = buff + 1;
@@ -350,6 +350,7 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
 			}
 		}
 	}
+	*len = strlen(*search);
 
 	return type;
 }
@@ -362,8 +363,7 @@ static void filter_build_regex(struct filter_pred *pred)
 	int not = 0;
 
 	if (pred->op == OP_GLOB) {
-		type = filter_parse_regex(r->pattern, r->len, &search, &not);
-		r->len = strlen(search);
+		type = filter_parse_regex(r->pattern, &r->len, &search, &not);
 		memmove(r->pattern, search, r->len+1);
 	}
 
-- 
2.1.4

[RFC][PATCH 6/7] trace: constify filter_parse_regex(), match_records(), ftrace_match() and ftrace_match_record()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

stop modifying the glob in there.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c              | 14 +++++++-------
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c |  8 ++++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e082681..572e3df 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3362,7 +3362,7 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 				 inode, file);
 }
 
-static int ftrace_match(char *str, char *regex, int len, int type)
+static int ftrace_match(char *str, const char *regex, int len, int type)
 {
 	int matched = 0;
 	int slen = strlen(str);
@@ -3417,7 +3417,7 @@ enter_record(struct ftrace_hash *hash, struct dyn_ftrace *rec, int not)
 
 static int
 ftrace_match_record(struct dyn_ftrace *rec, char *mod,
-		    char *regex, int len, int type)
+		    const char *regex, int len, int type)
 {
 	char str[KSYM_SYMBOL_LEN];
 	char *modname;
@@ -3438,13 +3438,13 @@ ftrace_match_record(struct dyn_ftrace *rec, char *mod,
 }
 
 static int
-match_records(struct ftrace_hash *hash, char *buff,
+match_records(struct ftrace_hash *hash, const char *buff,
 	      int len, char *mod, int not)
 {
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	int type = MATCH_FULL;
-	char *search = buff;
+	const char *search = buff;
 	int found = 0;
 	int ret;
 
@@ -3648,7 +3648,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	int type, len = strlen(glob), not;
 	unsigned long key;
 	int count = 0;
-	char *search;
+	const char *search;
 	int ret;
 
 	type = filter_parse_regex(glob, &len, &search, &not);
@@ -3759,7 +3759,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	char str[KSYM_SYMBOL_LEN];
 	int type = MATCH_FULL;
 	int i, len = 0;
-	char *search;
+	const char *search;
 	int ret;
 
 	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
@@ -4550,7 +4550,7 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 	int search_len = strlen(buffer);
 	int fail = 1;
 	int type, not;
-	char *search;
+	const char *search;
 	bool exists;
 	int i;
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7483205..09279c5 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1051,7 +1051,7 @@ struct filter_pred {
 };
 
 extern enum regex_type
-filter_parse_regex(char *buff, int *len, char **search, int *not);
+filter_parse_regex(const char *buff, int *len, const char **search, int *not);
 extern void print_event_filter(struct ftrace_event_file *file,
 			       struct trace_seq *s);
 extern int apply_event_filter(struct ftrace_event_file *file,
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 6a659e1..5cefdd8 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -321,7 +321,8 @@ static int regex_match_end(char *str, struct regex *r, int len)
  *  not returns 1 if buff started with a '!'
  *     0 otherwise.
  */
-enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
+enum regex_type filter_parse_regex(const char *buff, int *len,
+				   const char **search, int *not)
 {
 	int type = MATCH_FULL;
 	int i;
@@ -345,12 +346,11 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
 					type = MATCH_MIDDLE_ONLY;
 				else
 					type = MATCH_FRONT_ONLY;
-				buff[i] = 0;
 				break;
 			}
 		}
 	}
-	*len = strlen(*search);
+	*len = buff + i - *search;
 
 	return type;
 }
@@ -358,7 +358,7 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
 static void filter_build_regex(struct filter_pred *pred)
 {
 	struct regex *r = &pred->regex;
-	char *search;
+	const char *search;
 	enum regex_type type = MATCH_FULL;
 	int not = 0;
 
-- 
2.1.4

[RFC][PATCH 7/7] trace: constify glob arguments all way up to ftrace_function_set_regexp()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/ftrace.h             |  8 ++++----
 kernel/trace/ftrace.c              | 14 +++++++-------
 kernel/trace/trace_events_filter.c |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1da6029..ca33305 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -360,12 +360,12 @@ struct dyn_ftrace {
 int ftrace_force_update(void);
 int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
 			 int remove, int reset);
-int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_filter(struct ftrace_ops *ops, const unsigned char *buf,
 		       int len, int reset);
-int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_notrace(struct ftrace_ops *ops, const unsigned char *buf,
 			int len, int reset);
-void ftrace_set_global_filter(unsigned char *buf, int len, int reset);
-void ftrace_set_global_notrace(unsigned char *buf, int len, int reset);
+void ftrace_set_global_filter(const unsigned char *buf, int len, int reset);
+void ftrace_set_global_notrace(const unsigned char *buf, int len, int reset);
 void ftrace_free_filter(struct ftrace_ops *ops);
 
 int register_ftrace_command(struct ftrace_func_command *cmd);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 572e3df..17da223 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3473,7 +3473,7 @@ match_records(struct ftrace_hash *hash, const char *buff,
 }
 
 static int
-ftrace_match_records(struct ftrace_hash *hash, char *buff, int len)
+ftrace_match_records(struct ftrace_hash *hash, const char *buff, int len)
 {
 	return match_records(hash, buff, len, NULL, 0);
 }
@@ -4042,7 +4042,7 @@ static void ftrace_ops_update_code(struct ftrace_ops *ops,
 }
 
 static int
-ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
+ftrace_set_hash(struct ftrace_ops *ops, const unsigned char *buf, int len,
 		unsigned long ip, int remove, int reset, int enable)
 {
 	struct ftrace_hash **orig_hash;
@@ -4125,7 +4125,7 @@ int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
 EXPORT_SYMBOL_GPL(ftrace_set_filter_ip);
 
 static int
-ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
+ftrace_set_regex(struct ftrace_ops *ops, const unsigned char *buf, int len,
 		 int reset, int enable)
 {
 	return ftrace_set_hash(ops, buf, len, 0, 0, reset, enable);
@@ -4141,7 +4141,7 @@ ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
  * Filters denote which functions should be enabled when tracing is enabled.
  * If @buf is NULL and reset is set, all functions will be enabled for tracing.
  */
-int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_filter(struct ftrace_ops *ops, const unsigned char *buf,
 		       int len, int reset)
 {
 	ftrace_ops_init(ops);
@@ -4160,7 +4160,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_filter);
  * is enabled. If @buf is NULL and reset is set, all functions will be enabled
  * for tracing.
  */
-int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_notrace(struct ftrace_ops *ops, const unsigned char *buf,
 			int len, int reset)
 {
 	ftrace_ops_init(ops);
@@ -4176,7 +4176,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_notrace);
  * Filters denote which functions should be enabled when tracing is enabled.
  * If @buf is NULL and reset is set, all functions will be enabled for tracing.
  */
-void ftrace_set_global_filter(unsigned char *buf, int len, int reset)
+void ftrace_set_global_filter(const unsigned char *buf, int len, int reset)
 {
 	ftrace_set_regex(&global_ops, buf, len, reset, 1);
 }
@@ -4192,7 +4192,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_global_filter);
  * is enabled. If @buf is NULL and reset is set, all functions will be enabled
  * for tracing.
  */
-void ftrace_set_global_notrace(unsigned char *buf, int len, int reset)
+void ftrace_set_global_notrace(const unsigned char *buf, int len, int reset)
 {
 	ftrace_set_regex(&global_ops, buf, len, reset, 0);
 }
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5cefdd8..96ce797 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2098,7 +2098,7 @@ ftrace_function_filter_re(char *buf, int len, int *count)
 }
 
 static int ftrace_function_set_regexp(struct ftrace_ops *ops, int filter,
-				      int reset, char *re, int len)
+				      int reset, const char *re, int len)
 {
 	int ret;
 
-- 
2.1.4

Re: [RFC][PATCH 1/7] trace: fix the glob match in __unregister_ftrace_function_probe()

[Steven Rostedt]

On Thu,  5 Feb 2015 19:56:34 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> For patterns starting with '*' we need to match against 'search', not
> 'glob'.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  kernel/trace/ftrace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 224e768..9f90a4f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c

Can you make search = NULL instead of glob at the start of the
function. That is:

	if (!glob || strcmp(glob, "*") == 0 || !strlen(glob))
		search = NULL;

> @@ -3802,7 +3802,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
>  			if (glob) {

And then here have

			if (search) {

>  				kallsyms_lookup(entry->ip, NULL, NULL,
>  						NULL, str);
> -				if (!ftrace_match(str, glob, len, type))
> +				if (!ftrace_match(str, search, len, type))

Otherwise I'm sure there's a gcc out there that will give a warning
about search being used uninitialized.

-- Steve

>  					continue;
>  			}
>  

Re: [RFC][PATCH 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Steven Rostedt]

On Thu,  5 Feb 2015 19:56:38 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> ... by passing len by address and using it to report the length of
> substring in question.

You certainly are very verbose in your change logs.

What exactly is the purpose of this patch? Clean up? Optimization?

I can't really tell. Seems like you are just moving the strlen() from
outside the function into it.


> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1051,7 +1051,7 @@ struct filter_pred {
>  };
>  
>  extern enum regex_type
> -filter_parse_regex(char *buff, int len, char **search, int *not);
> +filter_parse_regex(char *buff, int *len, char **search, int *not);
>  extern void print_event_filter(struct ftrace_event_file *file,
>  			       struct trace_seq *s);
>  extern int apply_event_filter(struct ftrace_event_file *file,
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 6c4a96b..6a659e1 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -321,7 +321,7 @@ static int regex_match_end(char *str, struct regex *r, int len)
>   *  not returns 1 if buff started with a '!'
>   *     0 otherwise.
>   */

@len above needs to be updated for kernel doc. The description should
also note what *len gets set to at the end.

> -enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
> +enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
>  {
>  	int type = MATCH_FULL;
>  	int i;
> @@ -329,13 +329,13 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
>  	if (buff[0] == '!') {
>  		*not = 1;
>  		buff++;
> -		len--;
> +		(*len)--;
>  	} else
>  		*not = 0;
>  
>  	*search = buff;
>  
> -	for (i = 0; i < len; i++) {
> +	for (i = 0; i < *len; i++) {
>  		if (buff[i] == '*') {
>  			if (!i) {
>  				*search = buff + 1;
> @@ -350,6 +350,7 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
>  			}
>  		}
>  	}
> +	*len = strlen(*search);

We could optimize this even further:

	*len = &buff[i] - *search;

-- Steve

>  
>  	return type;
>  }

Re: [RFC][PATCH 6/7] trace: constify filter_parse_regex(), match_records(), ftrace_match() and ftrace_match_record()

[Steven Rostedt]

On Thu,  5 Feb 2015 19:56:39 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> @@ -345,12 +346,11 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
>  					type = MATCH_MIDDLE_ONLY;
>  				else
>  					type = MATCH_FRONT_ONLY;
> -				buff[i] = 0;
>  				break;
>  			}
>  		}
>  	}
> -	*len = strlen(*search);
> +	*len = buff + i - *search;

Heh, I guess I didn't need to mention this optimization. I would
recommend that this change goes into patch 5/7. No need to place it
here. It fits better with the previous patch.

-- Steve

>  
>  	return type;
>  }

Re: [RFC][PATCH 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Al Viro]

On Thu, Feb 05, 2015 at 04:29:33PM -0500, Steven Rostedt wrote:
> On Thu,  5 Feb 2015 19:56:38 +0000
> Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> > From: Al Viro <viro@zeniv.linux.org.uk>
> > 
> > ... by passing len by address and using it to report the length of
> > substring in question.
> 
> You certainly are very verbose in your change logs.
> 
> What exactly is the purpose of this patch? Clean up? Optimization?
> 
> I can't really tell. Seems like you are just moving the strlen() from
> outside the function into it.

The point is that by now this strlen() is the only thing for which we
NUL-termination of the substring; moving it inside the filter_parse_regex()
is an obviously equivalent transformation and it leaves that one strlen() call
inside filter_parse_regex() the only place where we still care about NUL.

The next commit kills it off completely, at which point we are done with
modifying the string at all.

Re: [RFC][PATCH 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Steven Rostedt]

On Thu, 5 Feb 2015 21:44:24 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:


> The point is that by now this strlen() is the only thing for which we
> NUL-termination of the substring; moving it inside the filter_parse_regex()
> is an obviously equivalent transformation and it leaves that one strlen() call
> inside filter_parse_regex() the only place where we still care about NUL.
> 
> The next commit kills it off completely, at which point we are done with
> modifying the string at all.

Thanks for the explanation,

Can you add that to the change log. I like to think patches can stand
on their own, and if they are added to help another patch, it should be
stated in the change log so someone doing a git blame followed by a git
show, knows what is going on.

Changes done for a patch series may be fine when that series is being
reviewed, but a change log is the only thing we have in the future to
understand why something was done. And a change done to help out
another change is lost when going back and looking at the history.

I've been burnt by my own code by looking back at why I did something
and not having a change log on a change describing things to why they
were done (because at the time it seemed obvious). But then I wasted a
lot of time digging through archives to figure out the history of some
change. I rather avoid doing that again.

Thanks,

-- Steve

Re: [RFC][PATCH 1/7] trace: fix the glob match in __unregister_ftrace_function_probe()

[Al Viro]

On Thu, Feb 05, 2015 at 03:43:36PM -0500, Steven Rostedt wrote:
> Can you make search = NULL instead of glob at the start of the
> function. That is:
> 
> 	if (!glob || strcmp(glob, "*") == 0 || !strlen(glob))
> 		search = NULL;

Umm...  If we do it that way, I'd rather initialized search with NULL
and inverted the test:
	if (glob && *glob && strcmp(glob, "*") != 0) {
		parse it and set search
	}

Re: [RFC][PATCH 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Al Viro]

On Thu, Feb 05, 2015 at 05:07:43PM -0500, Steven Rostedt wrote:
> On Thu, 5 Feb 2015 21:44:24 +0000
> Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> 
> > The point is that by now this strlen() is the only thing for which we
> > NUL-termination of the substring; moving it inside the filter_parse_regex()
> > is an obviously equivalent transformation and it leaves that one strlen() call
> > inside filter_parse_regex() the only place where we still care about NUL.
> > 
> > The next commit kills it off completely, at which point we are done with
> > modifying the string at all.
> 
> Thanks for the explanation,
> 
> Can you add that to the change log. I like to think patches can stand
> on their own, and if they are added to help another patch, it should be
> stated in the change log so someone doing a git blame followed by a git
> show, knows what is going on.

Done and force-pushed.  Do you want me to repost the whole thing?

Re: [RFC][PATCH 1/7] trace: fix the glob match in __unregister_ftrace_function_probe()

[Steven Rostedt]

On Thu, 5 Feb 2015 22:30:39 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Thu, Feb 05, 2015 at 03:43:36PM -0500, Steven Rostedt wrote:
> > Can you make search = NULL instead of glob at the start of the
> > function. That is:
> > 
> > 	if (!glob || strcmp(glob, "*") == 0 || !strlen(glob))
> > 		search = NULL;
> 
> Umm...  If we do it that way, I'd rather initialized search with NULL
> and inverted the test:
> 	if (glob && *glob && strcmp(glob, "*") != 0) {
> 		parse it and set search
> 	}

Fine with me.

-- Steve

[RFC][PATCH v2 1/7] trace: fix the glob match in __unregister_ftrace_function_probe()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

For patterns starting with '*' we need to match against 'search', not
'glob'.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 224e768..67ecd14 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3761,12 +3761,10 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	char str[KSYM_SYMBOL_LEN];
 	int type = MATCH_FULL;
 	int i, len = 0;
-	char *search;
+	char *search = NULL;
 	int ret;
 
-	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
-		glob = NULL;
-	else if (glob) {
+	if (glob && *glob && strcmp(glob, "*") != 0) {
 		int not;
 
 		type = filter_parse_regex(glob, strlen(glob), &search, &not);
@@ -3799,10 +3797,10 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 				continue;
 
 			/* do this last, since it is the most expensive */
-			if (glob) {
+			if (search) {
 				kallsyms_lookup(entry->ip, NULL, NULL,
 						NULL, str);
-				if (!ftrace_match(str, glob, len, type))
+				if (!ftrace_match(str, search, len, type))
 					continue;
 			}
 
-- 
2.1.4

[RFC][PATCH v2 2/7] implement memmem()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/string.h |  1 +
 lib/string.c           | 34 ++++++++++++++++++++++------------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 2e22a2e..87f9fba 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -75,6 +75,7 @@ extern char * strstr(const char *, const char *);
 #endif
 #ifndef __HAVE_ARCH_STRNSTR
 extern char * strnstr(const char *, const char *, size_t);
+extern void * memmem(const void *, size_t, const void *, 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 1006330..2035dbe 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -740,27 +740,37 @@ EXPORT_SYMBOL(strstr);
 #endif
 
 #ifndef __HAVE_ARCH_STRNSTR
+
 /**
- * strnstr - Find the first substring in a length-limited string
+ * memmem - Find the first length-limited substring in a length-limited string
  * @s1: The string to be searched
+ * @len1: the maximum number of characters to search
  * @s2: The string to search for
- * @len: the maximum number of characters to search
+ * @len2: the length of the string being searched
  */
-char *strnstr(const char *s1, const char *s2, size_t len)
+void *memmem(const void *s1, size_t len1, const void *s2, size_t len2)
 {
-	size_t l2;
-
-	l2 = strlen(s2);
-	if (!l2)
-		return (char *)s1;
-	while (len >= l2) {
-		len--;
-		if (!memcmp(s1, s2, l2))
-			return (char *)s1;
+	if (!len2)
+		return (void *)s1;
+	while (len1 >= len2) {
+		len1--;
+		if (!memcmp(s1, s2, len2))
+			return (void *)s1;
 		s1++;
 	}
 	return NULL;
 }
+EXPORT_SYMBOL(memmem);
+/**
+ * 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)
+{
+	return memmem(s1, len, s2, strlen(s2));
+}
 EXPORT_SYMBOL(strnstr);
 #endif
 
-- 
2.1.4

[RFC][PATCH v2 3/7] trace_events_filter.c: switch to memcmp() and memmem() for matching

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

we know the lengths in advance...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/trace_events_filter.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ced69da..6c4a96b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -271,21 +271,25 @@ static int filter_pred_none(struct filter_pred *pred, void *event)
 
 static int regex_match_full(char *str, struct regex *r, int len)
 {
-	if (strncmp(str, r->pattern, len) == 0)
+	if (len - 1 != r->len)
+		return 0;
+	if (memcmp(str, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }
 
 static int regex_match_front(char *str, struct regex *r, int len)
 {
-	if (strncmp(str, r->pattern, r->len) == 0)
+	if (len - 1 < r->len)
+		return 0;
+	if (memcmp(str, r->pattern, r->len) == 0)
 		return 1;
 	return 0;
 }
 
 static int regex_match_middle(char *str, struct regex *r, int len)
 {
-	if (strnstr(str, r->pattern, len))
+	if (memmem(str, len, r->pattern, r->len))
 		return 1;
 	return 0;
 }
-- 
2.1.4

[RFC][PATCH v2 4/7] ftrace: switch matching to memcmp() and memmem()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

now we don't need the glob substring being matched to get
NUL-terminated by filter_parse_regex()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 67ecd14..a369a54 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3365,24 +3365,26 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 static int ftrace_match(char *str, char *regex, int len, int type)
 {
 	int matched = 0;
-	int slen;
+	int slen = strlen(str);
+
+	if (slen < len)
+		return 0;
 
 	switch (type) {
 	case MATCH_FULL:
-		if (strcmp(str, regex) == 0)
+		if (slen == len && memcmp(str, regex, len) == 0)
 			matched = 1;
 		break;
 	case MATCH_FRONT_ONLY:
-		if (strncmp(str, regex, len) == 0)
+		if (memcmp(str, regex, len) == 0)
 			matched = 1;
 		break;
 	case MATCH_MIDDLE_ONLY:
-		if (strstr(str, regex))
+		if (memmem(str, slen, regex, len))
 			matched = 1;
 		break;
 	case MATCH_END_ONLY:
-		slen = strlen(str);
-		if (slen >= len && memcmp(str + slen - len, regex, len) == 0)
+		if (memcmp(str + slen - len, regex, len) == 0)
 			matched = 1;
 		break;
 	}
-- 
2.1.4

[RFC][PATCH v2 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

	By this point the only thing that cares about
NUL-termination of substring found by filter_parse_regex() is
strlen() done immediately by all its callers.  Once we have found
the length, that's it - we won't be looking at more than that
many bytes in the substring in question.

	This commit consolidates those strlen() calls into one
done by filter_parse_regex() itself.  To report the length to the
callers, we switch to passing 'len' argument of filter_parse_regex()
by address and use it to pass the length of our substring on the
way out.

	The next commit will eliminate modifying the string
completely - filter_parse_regex() has enough information to find
the length of the substring without bothering with this strlen()
call.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c              | 24 +++++++++---------------
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c | 10 +++++-----
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a369a54..f640458 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3441,7 +3441,6 @@ static int
 match_records(struct ftrace_hash *hash, char *buff,
 	      int len, char *mod, int not)
 {
-	unsigned search_len = 0;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	int type = MATCH_FULL;
@@ -3449,10 +3448,8 @@ match_records(struct ftrace_hash *hash, char *buff,
 	int found = 0;
 	int ret;
 
-	if (len) {
-		type = filter_parse_regex(buff, len, &search, &not);
-		search_len = strlen(search);
-	}
+	if (len)
+		type = filter_parse_regex(buff, &len, &search, &not);
 
 	mutex_lock(&ftrace_lock);
 
@@ -3460,7 +3457,7 @@ match_records(struct ftrace_hash *hash, char *buff,
 		goto out_unlock;
 
 	do_for_each_ftrace_rec(pg, rec) {
-		if (ftrace_match_record(rec, mod, search, search_len, type)) {
+		if (ftrace_match_record(rec, mod, search, len, type)) {
 			ret = enter_record(hash, rec, not);
 			if (ret < 0) {
 				found = ret;
@@ -3648,14 +3645,13 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	struct ftrace_hash *hash;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
-	int type, len, not;
+	int type, len = strlen(glob), not;
 	unsigned long key;
 	int count = 0;
 	char *search;
 	int ret;
 
-	type = filter_parse_regex(glob, strlen(glob), &search, &not);
-	len = strlen(search);
+	type = filter_parse_regex(glob, &len, &search, &not);
 
 	/* we do not support '!' for function probes */
 	if (WARN_ON(not))
@@ -3768,9 +3764,9 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 
 	if (glob && *glob && strcmp(glob, "*") != 0) {
 		int not;
+		len = strlen(glob);
 
-		type = filter_parse_regex(glob, strlen(glob), &search, &not);
-		len = strlen(search);
+		type = filter_parse_regex(glob, &len, &search, &not);
 
 		/* we do not support '!' for function probes */
 		if (WARN_ON(not))
@@ -4549,7 +4545,7 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 {
 	struct dyn_ftrace *rec;
 	struct ftrace_page *pg;
-	int search_len;
+	int search_len = strlen(buffer);
 	int fail = 1;
 	int type, not;
 	char *search;
@@ -4557,12 +4553,10 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 	int i;
 
 	/* decode regex */
-	type = filter_parse_regex(buffer, strlen(buffer), &search, &not);
+	type = filter_parse_regex(buffer, &search_len, &search, &not);
 	if (!not && *idx >= size)
 		return -EBUSY;
 
-	search_len = strlen(search);
-
 	mutex_lock(&ftrace_lock);
 
 	if (unlikely(ftrace_disabled)) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8de48ba..7483205 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1051,7 +1051,7 @@ struct filter_pred {
 };
 
 extern enum regex_type
-filter_parse_regex(char *buff, int len, char **search, int *not);
+filter_parse_regex(char *buff, int *len, char **search, int *not);
 extern void print_event_filter(struct ftrace_event_file *file,
 			       struct trace_seq *s);
 extern int apply_event_filter(struct ftrace_event_file *file,
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 6c4a96b..6a659e1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -321,7 +321,7 @@ static int regex_match_end(char *str, struct regex *r, int len)
  *  not returns 1 if buff started with a '!'
  *     0 otherwise.
  */
-enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
+enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
 {
 	int type = MATCH_FULL;
 	int i;
@@ -329,13 +329,13 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
 	if (buff[0] == '!') {
 		*not = 1;
 		buff++;
-		len--;
+		(*len)--;
 	} else
 		*not = 0;
 
 	*search = buff;
 
-	for (i = 0; i < len; i++) {
+	for (i = 0; i < *len; i++) {
 		if (buff[i] == '*') {
 			if (!i) {
 				*search = buff + 1;
@@ -350,6 +350,7 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
 			}
 		}
 	}
+	*len = strlen(*search);
 
 	return type;
 }
@@ -362,8 +363,7 @@ static void filter_build_regex(struct filter_pred *pred)
 	int not = 0;
 
 	if (pred->op == OP_GLOB) {
-		type = filter_parse_regex(r->pattern, r->len, &search, &not);
-		r->len = strlen(search);
+		type = filter_parse_regex(r->pattern, &r->len, &search, &not);
 		memmove(r->pattern, search, r->len+1);
 	}
 
-- 
2.1.4

[RFC][PATCH v2 6/7] trace: constify filter_parse_regex(), match_records(), ftrace_match() and ftrace_match_record()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Stop modifying the glob in filter_parse_regex(); the only reason
for doing that is one strlen() call done right after we'd stored
that NUL and we can find the length of substring directly -
we know where it starts and we know which byte were we trying to
replace with NUL.

With that done, we can make filter_parse_regex() take const char *
(and return const char * via *search).  And that immediately
propagates to match_records(), ftrace_match() and ftrace_match_record() -
they can take the glob as const char * now.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/trace/ftrace.c              | 14 +++++++-------
 kernel/trace/trace.h               |  2 +-
 kernel/trace/trace_events_filter.c |  8 ++++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index f640458..b5b0aca 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3362,7 +3362,7 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 				 inode, file);
 }
 
-static int ftrace_match(char *str, char *regex, int len, int type)
+static int ftrace_match(char *str, const char *regex, int len, int type)
 {
 	int matched = 0;
 	int slen = strlen(str);
@@ -3417,7 +3417,7 @@ enter_record(struct ftrace_hash *hash, struct dyn_ftrace *rec, int not)
 
 static int
 ftrace_match_record(struct dyn_ftrace *rec, char *mod,
-		    char *regex, int len, int type)
+		    const char *regex, int len, int type)
 {
 	char str[KSYM_SYMBOL_LEN];
 	char *modname;
@@ -3438,13 +3438,13 @@ ftrace_match_record(struct dyn_ftrace *rec, char *mod,
 }
 
 static int
-match_records(struct ftrace_hash *hash, char *buff,
+match_records(struct ftrace_hash *hash, const char *buff,
 	      int len, char *mod, int not)
 {
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	int type = MATCH_FULL;
-	char *search = buff;
+	const char *search = buff;
 	int found = 0;
 	int ret;
 
@@ -3648,7 +3648,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	int type, len = strlen(glob), not;
 	unsigned long key;
 	int count = 0;
-	char *search;
+	const char *search;
 	int ret;
 
 	type = filter_parse_regex(glob, &len, &search, &not);
@@ -3759,7 +3759,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
 	char str[KSYM_SYMBOL_LEN];
 	int type = MATCH_FULL;
 	int i, len = 0;
-	char *search = NULL;
+	const char *search = NULL;
 	int ret;
 
 	if (glob && *glob && strcmp(glob, "*") != 0) {
@@ -4548,7 +4548,7 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)
 	int search_len = strlen(buffer);
 	int fail = 1;
 	int type, not;
-	char *search;
+	const char *search;
 	bool exists;
 	int i;
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7483205..09279c5 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1051,7 +1051,7 @@ struct filter_pred {
 };
 
 extern enum regex_type
-filter_parse_regex(char *buff, int *len, char **search, int *not);
+filter_parse_regex(const char *buff, int *len, const char **search, int *not);
 extern void print_event_filter(struct ftrace_event_file *file,
 			       struct trace_seq *s);
 extern int apply_event_filter(struct ftrace_event_file *file,
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 6a659e1..5cefdd8 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -321,7 +321,8 @@ static int regex_match_end(char *str, struct regex *r, int len)
  *  not returns 1 if buff started with a '!'
  *     0 otherwise.
  */
-enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
+enum regex_type filter_parse_regex(const char *buff, int *len,
+				   const char **search, int *not)
 {
 	int type = MATCH_FULL;
 	int i;
@@ -345,12 +346,11 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
 					type = MATCH_MIDDLE_ONLY;
 				else
 					type = MATCH_FRONT_ONLY;
-				buff[i] = 0;
 				break;
 			}
 		}
 	}
-	*len = strlen(*search);
+	*len = buff + i - *search;
 
 	return type;
 }
@@ -358,7 +358,7 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
 static void filter_build_regex(struct filter_pred *pred)
 {
 	struct regex *r = &pred->regex;
-	char *search;
+	const char *search;
 	enum regex_type type = MATCH_FULL;
 	int not = 0;
 
-- 
2.1.4

[RFC][PATCH v2 7/7] trace: constify glob arguments all way up to ftrace_function_set_regexp()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/ftrace.h             |  8 ++++----
 kernel/trace/ftrace.c              | 14 +++++++-------
 kernel/trace/trace_events_filter.c |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1da6029..ca33305 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -360,12 +360,12 @@ struct dyn_ftrace {
 int ftrace_force_update(void);
 int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
 			 int remove, int reset);
-int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_filter(struct ftrace_ops *ops, const unsigned char *buf,
 		       int len, int reset);
-int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_notrace(struct ftrace_ops *ops, const unsigned char *buf,
 			int len, int reset);
-void ftrace_set_global_filter(unsigned char *buf, int len, int reset);
-void ftrace_set_global_notrace(unsigned char *buf, int len, int reset);
+void ftrace_set_global_filter(const unsigned char *buf, int len, int reset);
+void ftrace_set_global_notrace(const unsigned char *buf, int len, int reset);
 void ftrace_free_filter(struct ftrace_ops *ops);
 
 int register_ftrace_command(struct ftrace_func_command *cmd);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index b5b0aca..6035c08 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3473,7 +3473,7 @@ match_records(struct ftrace_hash *hash, const char *buff,
 }
 
 static int
-ftrace_match_records(struct ftrace_hash *hash, char *buff, int len)
+ftrace_match_records(struct ftrace_hash *hash, const char *buff, int len)
 {
 	return match_records(hash, buff, len, NULL, 0);
 }
@@ -4040,7 +4040,7 @@ static void ftrace_ops_update_code(struct ftrace_ops *ops,
 }
 
 static int
-ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
+ftrace_set_hash(struct ftrace_ops *ops, const unsigned char *buf, int len,
 		unsigned long ip, int remove, int reset, int enable)
 {
 	struct ftrace_hash **orig_hash;
@@ -4123,7 +4123,7 @@ int ftrace_set_filter_ip(struct ftrace_ops *ops, unsigned long ip,
 EXPORT_SYMBOL_GPL(ftrace_set_filter_ip);
 
 static int
-ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
+ftrace_set_regex(struct ftrace_ops *ops, const unsigned char *buf, int len,
 		 int reset, int enable)
 {
 	return ftrace_set_hash(ops, buf, len, 0, 0, reset, enable);
@@ -4139,7 +4139,7 @@ ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
  * Filters denote which functions should be enabled when tracing is enabled.
  * If @buf is NULL and reset is set, all functions will be enabled for tracing.
  */
-int ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_filter(struct ftrace_ops *ops, const unsigned char *buf,
 		       int len, int reset)
 {
 	ftrace_ops_init(ops);
@@ -4158,7 +4158,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_filter);
  * is enabled. If @buf is NULL and reset is set, all functions will be enabled
  * for tracing.
  */
-int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+int ftrace_set_notrace(struct ftrace_ops *ops, const unsigned char *buf,
 			int len, int reset)
 {
 	ftrace_ops_init(ops);
@@ -4174,7 +4174,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_notrace);
  * Filters denote which functions should be enabled when tracing is enabled.
  * If @buf is NULL and reset is set, all functions will be enabled for tracing.
  */
-void ftrace_set_global_filter(unsigned char *buf, int len, int reset)
+void ftrace_set_global_filter(const unsigned char *buf, int len, int reset)
 {
 	ftrace_set_regex(&global_ops, buf, len, reset, 1);
 }
@@ -4190,7 +4190,7 @@ EXPORT_SYMBOL_GPL(ftrace_set_global_filter);
  * is enabled. If @buf is NULL and reset is set, all functions will be enabled
  * for tracing.
  */
-void ftrace_set_global_notrace(unsigned char *buf, int len, int reset)
+void ftrace_set_global_notrace(const unsigned char *buf, int len, int reset)
 {
 	ftrace_set_regex(&global_ops, buf, len, reset, 0);
 }
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5cefdd8..96ce797 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2098,7 +2098,7 @@ ftrace_function_filter_re(char *buf, int len, int *count)
 }
 
 static int ftrace_function_set_regexp(struct ftrace_ops *ops, int filter,
-				      int reset, char *re, int len)
+				      int reset, const char *re, int len)
 {
 	int ret;
 
-- 
2.1.4

Re: [RFC][PATCH v2 2/7] implement memmem()

[Steven Rostedt]

On Fri,  6 Feb 2015 04:00:10 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  include/linux/string.h |  1 +
>  lib/string.c           | 34 ++++++++++++++++++++++------------
>  2 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 2e22a2e..87f9fba 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -75,6 +75,7 @@ extern char * strstr(const char *, const char *);
>  #endif
>  #ifndef __HAVE_ARCH_STRNSTR
>  extern char * strnstr(const char *, const char *, size_t);
> +extern void * memmem(const void *, size_t, const void *, 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 1006330..2035dbe 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -740,27 +740,37 @@ EXPORT_SYMBOL(strstr);
>  #endif
>  
>  #ifndef __HAVE_ARCH_STRNSTR

Although I do not see any archs that have this set, if one were to set
it, then memmem() would also have to be implemented.

Should this patch add a:

#ifndef __HAVE_ARCH_MEMMEM

?

-- Steve

> +
>  /**
> - * strnstr - Find the first substring in a length-limited string
> + * memmem - Find the first length-limited substring in a length-limited string
>   * @s1: The string to be searched
> + * @len1: the maximum number of characters to search
>   * @s2: The string to search for
> - * @len: the maximum number of characters to search
> + * @len2: the length of the string being searched
>   */
> -char *strnstr(const char *s1, const char *s2, size_t len)
> +void *memmem(const void *s1, size_t len1, const void *s2, size_t len2)
>  {

Re: [RFC][PATCH v2 3/7] trace_events_filter.c: switch to memcmp() and memmem() for matching

[Steven Rostedt]

On Fri,  6 Feb 2015 04:00:11 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> we know the lengths in advance...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  kernel/trace/trace_events_filter.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index ced69da..6c4a96b 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -271,21 +271,25 @@ static int filter_pred_none(struct filter_pred *pred, void *event)
>  
>  static int regex_match_full(char *str, struct regex *r, int len)
>  {
> -	if (strncmp(str, r->pattern, len) == 0)
> +	if (len - 1 != r->len)
> +		return 0;
> +	if (memcmp(str, r->pattern, r->len) == 0)
>  		return 1;
>  	return 0;
>  }
>  
>  static int regex_match_front(char *str, struct regex *r, int len)
>  {
> -	if (strncmp(str, r->pattern, r->len) == 0)
> +	if (len - 1 < r->len)
> +		return 0;
> +	if (memcmp(str, r->pattern, r->len) == 0)
>  		return 1;
>  	return 0;
>  }
>  
>  static int regex_match_middle(char *str, struct regex *r, int len)
>  {
> -	if (strnstr(str, r->pattern, len))
> +	if (memmem(str, len, r->pattern, r->len))

Since len includes '\0', shouldn't this be:

	if (memmem(str, len - 1, r->pattern, r->len);

It doesn't break the code either way, the algorithm still works, and
the original code did not pass in len-1 either. But if this is
optimizing the code, might as well eliminate one extra loop.

-- Steve

>  		return 1;
>  	return 0;
>  }

Re: [RFC][PATCH v2 5/7] trace: make filter_parse_regex() provide the length of substring to compare with

[Steven Rostedt]

On Fri,  6 Feb 2015 04:00:13 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:


> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -321,7 +321,7 @@ static int regex_match_end(char *str, struct regex *r, int len)
>   *  not returns 1 if buff started with a '!'
>   *     0 otherwise.
>   */

Still needs the update to kernel doc.

> -enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
> +enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not)
>  {
>  	int type = MATCH_FULL;
>  	int i;
> @@ -329,13 +329,13 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
>  	if (buff[0] == '!') {
>  		*not = 1;
>  		buff++;
> -		len--;
> +		(*len)--;
>  	} else
>  		*not = 0;
>  
>  	*search = buff;
>  
> -	for (i = 0; i < len; i++) {
> +	for (i = 0; i < *len; i++) {
>  		if (buff[i] == '*') {
>  			if (!i) {
>  				*search = buff + 1;
> @@ -350,6 +350,7 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)
>  			}
>  		}
>  	}
> +	*len = strlen(*search);

This should just add what the next patch does, instead of adding the
strlen() then changing it again. The change works here as well.

-	*len = strlen(*search);
+	*len = buff + i - *search;

It makes more sense to just add it here instead of having it changed
again.

-- Steve


>  
>  	return type;
>  }
> @@ -362,8 +363,7 @@ static void filter_build_regex(struct filter_pred *pred)
>  	int not = 0;
>  
>  	if (pred->op == OP_GLOB) {
> -		type = filter_parse_regex(r->pattern, r->len, &search, &not);
> -		r->len = strlen(search);
> +		type = filter_parse_regex(r->pattern, &r->len, &search, &not);
>  		memmove(r->pattern, search, r->len+1);
>  	}
>  

Re: [RFC][PATCH v2 6/7] trace: constify filter_parse_regex(), match_records(), ftrace_match() and ftrace_match_record()

[Steven Rostedt]

On Fri,  6 Feb 2015 04:00:14 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> +				   const char **search, int *not)
>  {
>  	int type = MATCH_FULL;
>  	int i;
> @@ -345,12 +346,11 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
>  					type = MATCH_MIDDLE_ONLY;
>  				else
>  					type = MATCH_FRONT_ONLY;
> -				buff[i] = 0;
>  				break;
>  			}
>  		}
>  	}
> -	*len = strlen(*search);
> +	*len = buff + i - *search;

This change belongs in the previous patch.

-- Steve

>  
>  	return type;
>  }
> @@ -358,7 +358,7 @@ enum regex_type filter_parse_regex(char *buff, int *len, char **search, int *not
>  static void filter_build_regex(struct filter_pred *pred)
>  {
>  	struct regex *r = &pred->regex;
> -	char *search;
> +	const char *search;
>  	enum regex_type type = MATCH_FULL;
>  	int not = 0;
>  

Re: [RFC][PATCH v2 2/7] implement memmem()

[Rasmus Villemoes]

On Fri, Feb 06 2015, Al Viro <viro@ZenIV.linux.org.uk> wrote:

> +/**
> + * 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)
> +{
> +	return memmem(s1, len, s2, strlen(s2));
> +}

Most strn* interfaces don't require the n to be at most the actual
string length, but it seems that this would happily search past the '\0'
of s1 if len is large enough, e.g.

strnstr("abc\0def", "def", 1000)

will not return NULL (unlike what the libbsd version does). So either
that restriction should be documented or len should be replaced by
min(len, strlen(s1)).

Rasmus

Re: [RFC][PATCH v2 2/7] implement memmem()

[Steven Rostedt]

On Fri, 06 Feb 2015 23:30:19 +0100
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> On Fri, Feb 06 2015, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> > +/**
> > + * 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)
> > +{
> > +	return memmem(s1, len, s2, strlen(s2));
> > +}
> 
> Most strn* interfaces don't require the n to be at most the actual
> string length, but it seems that this would happily search past the '\0'
> of s1 if len is large enough, e.g.
> 
> strnstr("abc\0def", "def", 1000)
> 
> will not return NULL (unlike what the libbsd version does). So either
> that restriction should be documented or len should be replaced by
> min(len, strlen(s1)).

The version that is currently in the kernel will do exactly what the
patched version will do. Although it may be different than what libbsd
might do, we don't really care. The kernel doc states that @len will be
the maximum number of characters to search, and that's exactly what
this does.

The functionality of strnstr() does not change with this patch, so it
does not need to address your concern.

Feel free to submit a patch that states the difference of the kernel
version of strnstr with whatever version is out in the wild. But that
is out of scope with this series.

-- Steve

Re: [RFC][PATCH v2 2/7] implement memmem()

[Rasmus Villemoes]

On Fri, Feb 06 2015, Steven Rostedt <rostedt@goodmis.org> wrote:

> The functionality of strnstr() does not change with this patch, so it
> does not need to address your concern.

Yeah, I should have checked that, of course. Sorry.

> Feel free to submit a patch that states the difference of the kernel
> version of strnstr with whatever version is out in the wild.

It seems a much better name would be memstr, and "length-limited string"
is misleading, since s1 is in no way treated as a string. But I'll leave
it for now.

Rasmus

Re: [RFC][PATCHES] constifying ftrace globs

[Steven Rostedt]

Seems that this series fell through the cracks. Is it worth
resurrecting?

-- Steve


On Thu, 5 Feb 2015 19:49:14 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> 	We don't really need to NUL-terminate the substring we are
> matching against; all it takes is introducing memmem(3) and using it
> instead of str[n]str().
> 
> 	It's not that much work - see vfs.git#ftrace_glob.  The reason
> I went there is rather amusing; it all started with making do_execve()
> take arrays of const strings for argv and envp.  After all, we never
> change them *and* we often enough pass arrays of string literals that
> way.  It exploded into a series of 75 commits, with the final ripple
> being argv_split() and argv_free().  OK, turns out that ftrace is
> using those as well, fortunately it's done to get arrays of regexes,
> so it should be trivial to constify as well, right?  Imagine the amount of
> swearing when I noticed that it *does* modify the resulting strings...
> 
> 	This series deals with that problem, providing the missing prereq for
> the patchbomb from hell...
> 
> 	Could you review #ftrace_glob?  It's not large - seven commits,
> boiling down to
>  include/linux/ftrace.h             |  8 ++---
>  include/linux/string.h             |  1 +
>  kernel/trace/ftrace.c              | 68 ++++++++++++++++++--------------------
>  kernel/trace/trace.h               |  2 +-
>  kernel/trace/trace_events_filter.c | 26 +++++++++------
>  lib/string.c                       | 34 ++++++++++++-------
>  6 files changed, 75 insertions(+), 64 deletions(-)
> 
> Patches in followups.