linux-kernel - [PATCH] tracing: Fix possible memory leak in ftrace_regsiter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Wed, 27 Dec 2023 21:38:25 +0900
From: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>,
	Jiri Olsa <jolsa@...nel.org>
Cc: linux-trace-kernel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] tracing: Fix possible memory leak in ftrace_regsiter_direct()

From: Masami Hiramatsu (Google) <mhiramat@...nel.org>

If ftrace_register_direct() called with a large number of target
functions (e.g. 65), the free_hash can be updated twice or more
in the ftrace_add_rec_direct() without freeing the previous hash
memory. Thus this can cause a memory leak.

Fix this issue by expanding the direct_hash at once before
adding the new entries.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Fixes: f64dd4627ec6 ("ftrace: Add multi direct register/unregister interface")
Cc: stable@...r.kernel.org
---
 kernel/trace/ftrace.c |   49 +++++++++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 8de8bec5f366..9269c2c3e595 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2555,28 +2555,33 @@ unsigned long ftrace_find_rec_direct(unsigned long ip)
 	return entry->direct;
 }
 
-static struct ftrace_func_entry*
-ftrace_add_rec_direct(unsigned long ip, unsigned long addr,
-		      struct ftrace_hash **free_hash)
+static struct ftrace_hash *ftrace_expand_direct(int inc_count)
 {
-	struct ftrace_func_entry *entry;
+	struct ftrace_hash *new_hash, *free_hash;
+	int size = ftrace_hash_empty(direct_functions) ? 0 :
+		direct_functions->count + inc_count;
 
-	if (ftrace_hash_empty(direct_functions) ||
-	    direct_functions->count > 2 * (1 << direct_functions->size_bits)) {
-		struct ftrace_hash *new_hash;
-		int size = ftrace_hash_empty(direct_functions) ? 0 :
-			direct_functions->count + 1;
+	if (!ftrace_hash_empty(direct_functions) &&
+	    size <= 2 * (1 << direct_functions->size_bits))
+		return NULL;
 
-		if (size < 32)
-			size = 32;
+	if (size < 32)
+		size = 32;
 
-		new_hash = dup_hash(direct_functions, size);
-		if (!new_hash)
-			return NULL;
+	new_hash = dup_hash(direct_functions, size);
+	if (!new_hash)
+		return ERR_PTR(-ENOMEM);
 
-		*free_hash = direct_functions;
-		direct_functions = new_hash;
-	}
+	free_hash = direct_functions;
+	direct_functions = new_hash;
+
+	return free_hash;
+}
+
+static struct ftrace_func_entry*
+ftrace_add_rec_direct(unsigned long ip, unsigned long addr)
+{
+	struct ftrace_func_entry *entry;
 
 	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
@@ -5436,11 +5441,19 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
 		}
 	}
 
+	/* ... and prepare the insertion */
+	free_hash = ftrace_expand_direct(hash->count);
+	if (IS_ERR(free_hash)) {
+		err = PTR_ERR(free_hash);
+		free_hash = NULL;
+		goto out_unlock;
+	}
+
 	/* ... and insert them to direct_functions hash. */
 	err = -ENOMEM;
 	for (i = 0; i < size; i++) {
 		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
-			new = ftrace_add_rec_direct(entry->ip, addr, &free_hash);
+			new = ftrace_add_rec_direct(entry->ip, addr);
 			if (!new)
 				goto out_remove;
 			entry->direct = addr;