lkml.org 
[lkml]   [2018]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subject[RFC] apparent bogosity in unregister_ftrace_function_probe_func()
It contains something very odd:

func_g.type = filter_parse_regex(glob, strlen(glob),
&func_g.search, &not);
func_g.len = strlen(func_g.search);
func_g.search = glob;

/* we do not support '!' for function probes */
if (WARN_ON(not))
return -EINVAL;

What the hell is the last assignment for? After that call of
filter_parse_regex() we could have func_g.search not equal to glob
only if glob started with '!' or '*'. In the former case we would've
buggered off with -EINVAL (not = 1). In the latter we would've set
func_g.search equal to glob + 1, calculated the length of that thing
in func_g.len and proceeded to reset func_g.search back to glob.

Suppose the glob is e.g. *foo*. We end up with
func_g.type = MATCH_MIDDLE_ONLY;
func_g.len = 3;
func_g.search = "*foo";
Feeding that to ftrace_match_record() will not do anything sane - we
will be looking for names containing "*foo" (->len is ignored for that
one).

Incidentally, shouldn't filter_parse_regex("*[ab]", 5, &s, &not)
end up with s = "*[ab]"? We are returning MATCH_GLOB, after all,
so we want the entire pattern there... I would've assumed that
this is what the code in unregister_ftrace_function_probe_func()
is trying to compensate for, the first oddity predates MATCH_GLOB...

In any case, that should be done in filter_parse_regex() itself -
there are other callers that don't have such compensation and
it does the wrong thing for MATCH_MIDDLE_ONLY and MATCH_END_ONLY
cases...

That started in commit 3ba009297149fa45956c33ab5de7c5f4da1f28b8
Author: Dmitry Safonov <0x7f454c46@gmail.com>
Date: Tue Sep 29 19:46:14 2015 +0300

ftrace: Introduce ftrace_glob structure

without any explanation -
- type = filter_parse_regex(glob, strlen(glob), &search, &not);
- len = strlen(search);
+ func_g.type = filter_parse_regex(glob, strlen(glob),
+ &func_g.search, &not);
+ func_g.len = strlen(func_g.search);
+ func_g.search = glob;

Note in the same commit
- type = filter_parse_regex(glob, strlen(glob), &search, &not);
- len = strlen(search);
+ func_g.type = filter_parse_regex(glob, strlen(glob),
+ &func_g.search, &not);
+ func_g.len = strlen(func_g.search);
nearby (in register_ftrace_function_probe()).

What am I missing here?

\
 
 \ /
  Last update: 2018-01-27 04:17    [W:0.072 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site