From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751647AbeA0DRL (ORCPT <rfc822;w@1wt.eu>);
        Fri, 26 Jan 2018 22:17:11 -0500
Received: from zeniv.linux.org.uk ([195.92.253.2]:56676 "EHLO
        ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751398AbeA0DRK (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 26 Jan 2018 22:17:10 -0500
Date: Sat, 27 Jan 2018 03:17:06 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>, linux-kernel@vger.kernel.org
Subject: [RFC] apparent bogosity in unregister_ftrace_function_probe_func()
Message-ID: <20180127031706.GE13338@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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?