From: Masami Hiramatsu (Google) <[email protected]>
When register_fprobe_ips() fails, it tries to remove a list of
fprobe_hash_node from fprobe_ip_table, but it missed to remove
fprobe itself from fprobe_table. Moreover, when removing
the fprobe_hash_node which is added to rhltable once, it must
use kfree_rcu() after removing from rhltable.
To fix these issues, this reuses unregister_fprobe() internal
code to rollback the half-way registered fprobe.
Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer")
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
Changes in v3:
- Newly added.
---
kernel/trace/fprobe.c | 73 ++++++++++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 56d145017902..1b5c47685186 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -4,6 +4,7 @@
*/
#define pr_fmt(fmt) "fprobe: " fmt
+#include <linux/cleanup.h>
#include <linux/err.h>
#include <linux/fprobe.h>
#include <linux/kallsyms.h>
@@ -821,6 +822,8 @@ int register_fprobe(struct fprobe *fp, const char *filter,
const char *notfilter
}
EXPORT_SYMBOL_GPL(register_fprobe);
+static int unregister_fprobe_nolock(struct fprobe *fp);
+
/**
* register_fprobe_ips() - Register fprobe to ftrace by address.
* @fp: A fprobe data structure to be registered.
@@ -845,30 +848,29 @@ int register_fprobe_ips(struct fprobe *fp, unsigned long
*addrs, int num)
mutex_lock(&fprobe_mutex);
- hlist_array = fp->hlist_array;
if (fprobe_is_ftrace(fp))
ret = fprobe_ftrace_add_ips(addrs, num);
else
ret = fprobe_graph_add_ips(addrs, num);
+ if (ret) {
+ fprobe_fail_cleanup(fp);
+ return ret;
+ }
if (!ret) {
+ hlist_array = fp->hlist_array;
add_fprobe_hash(fp);
for (i = 0; i < hlist_array->size; i++) {
ret = insert_fprobe_node(&hlist_array->array[i]);
- if (ret)
+ if (ret) {
+ hlist_array->size = i;
+ unregister_fprobe_nolock(fp);
break;
- }
- /* fallback on insert error */
- if (ret) {
- for (i--; i >= 0; i--)
- delete_fprobe_node(&hlist_array->array[i]);
+ }
}
}
mutex_unlock(&fprobe_mutex);
- if (ret)
- fprobe_fail_cleanup(fp);
-
return ret;
}
EXPORT_SYMBOL_GPL(register_fprobe_ips);
@@ -911,32 +913,21 @@ bool fprobe_is_registered(struct fprobe *fp)
return true;
}
-/**
- * unregister_fprobe() - Unregister fprobe.
- * @fp: A fprobe data structure to be unregistered.
- *
- * Unregister fprobe (and remove ftrace hooks from the function entries).
- *
- * Return 0 if @fp is unregistered successfully, -errno if not.
- */
-int unregister_fprobe(struct fprobe *fp)
+static int unregister_fprobe_nolock(struct fprobe *fp)
{
- struct fprobe_hlist *hlist_array;
+ struct fprobe_hlist *hlist_array = fp->hlist_array;
unsigned long *addrs = NULL;
- int ret = 0, i, count;
+ int i, count;
- mutex_lock(&fprobe_mutex);
- if (!fp || !is_fprobe_still_exist(fp)) {
- ret = -EINVAL;
- goto out;
+ if (!hlist_array || !hlist_array->size) {
+ del_fprobe_hash(fp);
+ fprobe_fail_cleanup(fp);
+ return 0;
}
- hlist_array = fp->hlist_array;
addrs = kcalloc(hlist_array->size, sizeof(unsigned long), GFP_KERNEL);
- if (!addrs) {
- ret = -ENOMEM; /* TODO: Fallback to one-by-one loop */
- goto out;
- }
+ if (!addrs)
+ return -ENOMEM; /* TODO: Fallback to one-by-one loop */
/* Remove non-synonim ips from table and hash */
count = 0;
@@ -953,12 +944,26 @@ int unregister_fprobe(struct fprobe *fp)
kfree_rcu(hlist_array, rcu);
fp->hlist_array = NULL;
+ kfree(addrs);
-out:
- mutex_unlock(&fprobe_mutex);
+ return 0;
+}
- kfree(addrs);
- return ret;
+/**
+ * unregister_fprobe() - Unregister fprobe.
+ * @fp: A fprobe data structure to be unregistered.
+ *
+ * Unregister fprobe (and remove ftrace hooks from the function entries).
+ *
+ * Return 0 if @fp is unregistered successfully, -errno if not.
+ */
+int unregister_fprobe(struct fprobe *fp)
+{
+ guard(mutex)(&fprobe_mutex);
+ if (!fp || !is_fprobe_still_exist(fp))
+ return -EINVAL;
+
+ return unregister_fprobe_nolock(fp);
}
EXPORT_SYMBOL_GPL(unregister_fprobe);
