[PATCH -next 0/2] ftrace: Some minor fixes for set_ftrace_{notrace,filter} file handling

[PATCH -next 0/2] ftrace: Some minor fixes for set_ftrace_{notrace,filter} file handling

[Tengda Wu]

Hello,

This patch series addresses two issues in the ftrace subsystem related
to the handling of set_ftrace_{notrace,filter} files.

Patch 1 removes an incorrect free_ftrace_hash call in ftrace_regex_open()
that could lead to use-after-free when seq_open fails.

Patch 2 fixes a potential use-after-free scenario that can occur during
concurrent read/write operations on the set_ftrace_{notrace,filter} files.

Both issues were reproducible. This patches aims to fix them.

Best regards,
Tengda

Tengda Wu (2):
  ftrace: Remove unnecessary free_ftrace_hash in ftrace_regex_open
  ftrace: Fix potential use-after-free for set_ftrace_{notrace,filter}
    files

 kernel/trace/ftrace.c | 28 ++++++++++++++++++++++++----
 kernel/trace/trace.h  |  2 ++
 2 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH -next 1/2] ftrace: Remove unnecessary free_ftrace_hash in ftrace_regex_open

[Tengda Wu]

When reading set_ftrace_{notrace,filter} files, the ftrace_regex_open()
function incorrectly frees iter->hash when seq_open fails. This is
problematic because iter->hash points to the global {notrace,filter}_hash,
and the reading process freeing it would cause errors when other processes
access this memory.

The specific scenario occurs when:
1. {notrace,filter}_hash gets freed by a reader
2. Another process attempts to append something to the file
3. alloc_and_copy_ftrace_hash() tries to duplicate the freed {notrace,filter}_hash
4. A use-after-free (UAF) occurs

[  146.788584] BUG: KASAN: slab-use-after-free in alloc_and_copy_ftrace_hash+0x4b4/0x4c0
[  146.790278] Read of size 8 at addr ffff88810fa20818 by task bash/163
[  146.791254]
[  146.792776] CPU: 3 UID: 0 PID: 163 Comm: bash Not tainted 6.16.0-next-20250808+ #13 PREEMPT(full)
[  146.793243] Call Trace:
[  146.793393]  <TASK>
[  146.793572]  dump_stack_lvl+0x55/0x70
[  146.793729]  print_report+0xcb/0x610
[  146.793777]  ? __virt_addr_valid+0x1e8/0x2e0
[  146.793818]  ? alloc_and_copy_ftrace_hash+0x4b4/0x4c0
[  146.793852]  kasan_report+0xb8/0xf0
[  146.793886]  ? alloc_and_copy_ftrace_hash+0x4b4/0x4c0
[  146.793928]  alloc_and_copy_ftrace_hash+0x4b4/0x4c0
[  146.793964]  ? mutex_lock+0x90/0xe0
[  146.794002]  ftrace_regex_open+0x61f/0xc80
[  146.794182]  do_dentry_open+0x44f/0x1110
[  146.794369]  vfs_open+0x79/0x350
[  146.794412]  path_openat+0x13fb/0x3ec0
[  146.794662]  do_filp_open+0x1d7/0x420
[  146.794874]  do_sys_openat2+0xd4/0x160
[  146.794976]  __x64_sys_openat+0x122/0x1e0
[  146.795078]  do_syscall_64+0x4d/0x1d0
[  146.795112]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

The free_ftrace_hash call is just unnecessary in this context since
we shouldn't free the global hash that we don't own. Remove this call
to fix the issue.

Fixes: c20489dad156 ("ftrace: Assign iter->hash to filter or notrace hashes on seq read")
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
 kernel/trace/ftrace.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 00b76d450a89..cade13595b08 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4680,7 +4680,6 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
 			m->private = iter;
 		} else {
 			/* Failed */
-			free_ftrace_hash(iter->hash);
 			trace_parser_put(&iter->parser);
 		}
 	} else
-- 
2.34.1

Re: [PATCH -next 1/2] ftrace: Remove unnecessary free_ftrace_hash in ftrace_regex_open

[Steven Rostedt]

On Wed, 13 Aug 2025 02:30:43 +0000
Tengda Wu <wutengda@huaweicloud.com> wrote:


> The free_ftrace_hash call is just unnecessary in this context since
> we shouldn't free the global hash that we don't own. Remove this call
> to fix the issue.

This is incorrect as it is only unnecessary if it's a read.

The code above is:

	if (file->f_mode & FMODE_WRITE) {
		const int size_bits = FTRACE_HASH_DEFAULT_BITS;

		if (file->f_flags & O_TRUNC) {
			iter->hash = alloc_ftrace_hash(size_bits);
			clear_ftrace_mod_list(mod_head);
	        } else {
			iter->hash = alloc_and_copy_ftrace_hash(size_bits, hash);
		}

		if (!iter->hash) {
			trace_parser_put(&iter->parser);
			goto out_unlock;
		}
	} else
		iter->hash = hash;

Where if the file was open for write, the hash must be freed.


> 
> Fixes: c20489dad156 ("ftrace: Assign iter->hash to filter or notrace hashes on seq read")


> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
>  kernel/trace/ftrace.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 00b76d450a89..cade13595b08 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4680,7 +4680,6 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
>  			m->private = iter;
>  		} else {
>  			/* Failed */
> -			free_ftrace_hash(iter->hash);

The real fix is:

			if (file->f_mode & FMODE_WRITE)
				free_ftrace_hash(iter->hash);

-- Steve



>  			trace_parser_put(&iter->parser);
>  		}
>  	} else

[PATCH -next 2/2] ftrace: Fix potential use-after-free for set_ftrace_{notrace,filter} files

[Tengda Wu]

Concurrent read/write operations on the set_ftrace_{notrace,filter}
files may probabilistically trigger the following issues:

[ 2715.745293] BUG: unable to handle page fault for address: 00000003da393970
[ 2715.753736] CPU: 1 UID: 0 PID: 1324 Comm: read Not tainted 6.16.0-next-20250808 #1 PREEMPT(full)
[ 2715.755292] RIP: 0010:ftrace_lookup_ip+0x40/0x70
[ 2715.761114] Call Trace:
[ 2715.761462]  <TASK>
[ 2715.761705]  t_func_next.isra.0+0xaa/0xd0
[ 2715.762049]  t_start+0xa3/0x140
[ 2715.762207]  seq_read_iter+0xe8/0x4a0
[ 2715.762564]  seq_read+0x101/0x140
[ 2715.762769]  vfs_read+0xbd/0x340
[ 2715.763014]  ? preempt_count_add+0x4b/0xa0
[ 2715.763311]  ? do_sys_openat2+0x8c/0xd0
[ 2715.763623]  ksys_read+0x65/0xe0
[ 2715.763797]  do_syscall_64+0x4e/0x1c0
[ 2715.764049]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

The issue can be reproduced with the following script (using the
set_ftrace_notrace file as an example):

  while true; do
    echo __probestub_initcall_level > /sys/kernel/tracing/set_ftrace_notrace &
    cat /sys/kernel/tracing/set_ftrace_notrace &
  done

The root cause is that ftrace_regex_open and ftrace_regex_release
do not properly handle concurrent synchronization for notrace_hash.
Consider a race scenario between a reader and a writer:

1. The reader first obtains the value of notrace_hash via
   ftrace_regex_open().
2. The writer then updates notrace_hash via ftrace_regex_release()
   and frees the memory pointed to by the old notrace_hash.
3. Later, the reader accesses the old notrace_hash memory while
   ftrace_hash_empty() and ftrace_lookup_ip(), leading to a UAF.

CPU 1 (read)                                    CPU 2 (write)
ftrace_regex_open
  hash = ops->func_hash->notrace_hash;
  iter->hash = hash;
                                                ftrace_regex_open
                                                ftrace_regex_release
                                                  orig_hash = &iter->ops->func_hash->notrace_hash;
                                                  old_hash = *orig_hash;
                                                  free_ftrace_hash_rcu(old_hash);
t_start
  ftrace_hash_empty(iter->hash)
  t_func_next
    !ftrace_lookup_ip(iter->hash, rec->ip)

Since the reader's hash is always tied to its file descriptor (fd),
the writer cannot directly manage the reader's hash. To fix this,
introduce a refcount for ftrace_hash, initialized to 1. The count
is incremented only when a reader opens it, and decremented when
either a reader or writer releases it, thereby controlling the timing
of ftrace_hash deallocation.

Fixes: c20489dad156 ("ftrace: Assign iter->hash to filter or notrace hashes on seq read")
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
 kernel/trace/ftrace.c | 27 ++++++++++++++++++++++++---
 kernel/trace/trace.h  |  2 ++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cade13595b08..be4842054254 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1060,6 +1060,7 @@ struct ftrace_func_probe {
 static const struct hlist_head empty_buckets[1];
 static const struct ftrace_hash empty_hash = {
 	.buckets = (struct hlist_head *)empty_buckets,
+	.refcount = REFCOUNT_INIT(1),
 };
 #define EMPTY_HASH	((struct ftrace_hash *)&empty_hash)
 
@@ -1282,6 +1283,22 @@ static void free_ftrace_hash_rcu(struct ftrace_hash *hash)
 	call_rcu(&hash->rcu, __free_ftrace_hash_rcu);
 }
 
+static void get_ftrace_hash(struct ftrace_hash *hash)
+{
+	if (!hash || hash == EMPTY_HASH)
+		return;
+	if (!refcount_inc_not_zero(&hash->refcount))
+		WARN_ON(1);
+}
+
+static void put_ftrace_hash_rcu(struct ftrace_hash *hash)
+{
+	if (!hash || hash == EMPTY_HASH)
+		return;
+	if (refcount_dec_and_test(&hash->refcount))
+		call_rcu(&hash->rcu, __free_ftrace_hash_rcu);
+}
+
 /**
  * ftrace_free_filter - remove all filters for an ftrace_ops
  * @ops: the ops to remove the filters from
@@ -1316,6 +1333,7 @@ static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
 	}
 
 	hash->size_bits = size_bits;
+	refcount_set(&hash->refcount, 1);
 
 	return hash;
 }
@@ -3362,7 +3380,7 @@ static int __ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
 	ret = ftrace_hash_move(ops, enable, orig_hash, hash);
 	if (!ret) {
 		ftrace_ops_update_code(ops, &old_hash_ops);
-		free_ftrace_hash_rcu(old_hash);
+		put_ftrace_hash_rcu(old_hash);
 	}
 	return ret;
 }
@@ -3714,7 +3732,7 @@ static int ftrace_hash_move_and_update_subops(struct ftrace_ops *subops,
 		*orig_subhash = save_hash;
 		free_ftrace_hash_rcu(new_hash);
 	} else {
-		free_ftrace_hash_rcu(save_hash);
+		put_ftrace_hash_rcu(save_hash);
 	}
 	return ret;
 }
@@ -4666,8 +4684,10 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
 			trace_parser_put(&iter->parser);
 			goto out_unlock;
 		}
-	} else
+	} else {
 		iter->hash = hash;
+		get_ftrace_hash(iter->hash);
+	}
 
 	ret = 0;
 
@@ -6544,6 +6564,7 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
 		mutex_unlock(&ftrace_lock);
 	} else {
 		/* For read only, the hash is the ops hash */
+		put_ftrace_hash_rcu(iter->hash);
 		iter->hash = NULL;
 	}
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 1dbf1d3cf2f1..4936cd218c36 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -22,6 +22,7 @@
 #include <linux/ctype.h>
 #include <linux/once_lite.h>
 #include <linux/ftrace_regs.h>
+#include <linux/refcount.h>
 
 #include "pid_list.h"
 
@@ -905,6 +906,7 @@ struct ftrace_hash {
 	unsigned long		count;
 	unsigned long		flags;
 	struct rcu_head		rcu;
+	refcount_t		refcount;
 };
 
 struct ftrace_func_entry *
-- 
2.34.1

Re: [PATCH -next 2/2] ftrace: Fix potential use-after-free for set_ftrace_{notrace,filter} files

[Steven Rostedt]

On Wed, 13 Aug 2025 02:30:44 +0000
Tengda Wu <wutengda@huaweicloud.com> wrote:


> Since the reader's hash is always tied to its file descriptor (fd),
> the writer cannot directly manage the reader's hash. To fix this,
> introduce a refcount for ftrace_hash, initialized to 1. The count
> is incremented only when a reader opens it, and decremented when
> either a reader or writer releases it, thereby controlling the timing
> of ftrace_hash deallocation.

Hmm, I think the code that the first patch touches is the issue here too.

Instead of doing all these extra hacks, we should simply copy the hash for
read too.

That is, the real fix for both patches is this:

-- Steve

From: Steven Rostedt <rostedt@goodmis.org>
Subject: [PATCH] ftrace: Also allocate hash for reading of filter files

Currently the reader of set_ftrace_filter and set_ftrace_notrace just adds
the pointer to the global tracer hash to its iterator. Unlike the writer
that allocates a copy of the hash, the reader keeps the pointer to the
filter hashes. This is problematic because this pointer is static across
function calls that release the locks that can update the global tracer
hashes. This can cause UAF and similar bugs.

Allocate the hash for reading the filter files like it is done for the
writers. This not only fixes UAF bugs, but also makes the code a bit
simpler as it doesn't have to differentiate when to free the iterator's
hash between writers and readers.

Cc: stable@vger.kernel.org
Fixes: c20489dad156 ("ftrace: Assign iter->hash to filter or notrace hashes on seq read")
Closes: https://lore.kernel.org/all/20250813023044.2121943-1-wutengda@huaweicloud.com/
Reported-by: Tengda Wu <wutengda@huaweicloud.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 00b76d450a89..f992a5eb878e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4661,13 +4661,14 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
 	        } else {
 			iter->hash = alloc_and_copy_ftrace_hash(size_bits, hash);
 		}
+	} else {
+		iter->hash = alloc_and_copy_ftrace_hash(hash->size_bits, hash);
+	}
 
-		if (!iter->hash) {
-			trace_parser_put(&iter->parser);
-			goto out_unlock;
-		}
-	} else
-		iter->hash = hash;
+	if (!iter->hash) {
+		trace_parser_put(&iter->parser);
+		goto out_unlock;
+	}
 
 	ret = 0;
 
@@ -6543,9 +6544,6 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
 		ftrace_hash_move_and_update_ops(iter->ops, orig_hash,
 						      iter->hash, filter_hash);
 		mutex_unlock(&ftrace_lock);
-	} else {
-		/* For read only, the hash is the ops hash */
-		iter->hash = NULL;
 	}
 
 	mutex_unlock(&iter->ops->func_hash->regex_lock);
-- 
2.50.1

Re: [PATCH -next 2/2] ftrace: Fix potential use-after-free for set_ftrace_{notrace,filter} files

[Tengda Wu]

On 2025/8/20 9:05, Steven Rostedt wrote:
> On Wed, 13 Aug 2025 02:30:44 +0000
> Tengda Wu <wutengda@huaweicloud.com> wrote:
> 
> 
>> Since the reader's hash is always tied to its file descriptor (fd),
>> the writer cannot directly manage the reader's hash. To fix this,
>> introduce a refcount for ftrace_hash, initialized to 1. The count
>> is incremented only when a reader opens it, and decremented when
>> either a reader or writer releases it, thereby controlling the timing
>> of ftrace_hash deallocation.
> 
> Hmm, I think the code that the first patch touches is the issue here too.
> 
> Instead of doing all these extra hacks, we should simply copy the hash for
> read too.
> 
> That is, the real fix for both patches is this:
> 
> -- Steve
> 
> From: Steven Rostedt <rostedt@goodmis.org>
> Subject: [PATCH] ftrace: Also allocate hash for reading of filter files
> 
> Currently the reader of set_ftrace_filter and set_ftrace_notrace just adds
> the pointer to the global tracer hash to its iterator. Unlike the writer
> that allocates a copy of the hash, the reader keeps the pointer to the
> filter hashes. This is problematic because this pointer is static across
> function calls that release the locks that can update the global tracer
> hashes. This can cause UAF and similar bugs.
> 
> Allocate the hash for reading the filter files like it is done for the
> writers. This not only fixes UAF bugs, but also makes the code a bit
> simpler as it doesn't have to differentiate when to free the iterator's
> hash between writers and readers.

Agreed. That is a much cleaner solution. I just tested this code and it
worked perfectly. Looking forward to getting it into the mainline soon.

-- Tengda

> 
> Cc: stable@vger.kernel.org
> Fixes: c20489dad156 ("ftrace: Assign iter->hash to filter or notrace hashes on seq read")
> Closes: https://lore.kernel.org/all/20250813023044.2121943-1-wutengda@huaweicloud.com/
> Reported-by: Tengda Wu <wutengda@huaweicloud.com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  kernel/trace/ftrace.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 00b76d450a89..f992a5eb878e 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4661,13 +4661,14 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
>  	        } else {
>  			iter->hash = alloc_and_copy_ftrace_hash(size_bits, hash);
>  		}
> +	} else {
> +		iter->hash = alloc_and_copy_ftrace_hash(hash->size_bits, hash);
> +	}
>  
> -		if (!iter->hash) {
> -			trace_parser_put(&iter->parser);
> -			goto out_unlock;
> -		}
> -	} else
> -		iter->hash = hash;
> +	if (!iter->hash) {
> +		trace_parser_put(&iter->parser);
> +		goto out_unlock;
> +	}
>  
>  	ret = 0;
>  
> @@ -6543,9 +6544,6 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
>  		ftrace_hash_move_and_update_ops(iter->ops, orig_hash,
>  						      iter->hash, filter_hash);
>  		mutex_unlock(&ftrace_lock);
> -	} else {
> -		/* For read only, the hash is the ops hash */
> -		iter->hash = NULL;
>  	}
>  
>  	mutex_unlock(&iter->ops->func_hash->regex_lock);

Re: [PATCH -next 2/2] ftrace: Fix potential use-after-free for set_ftrace_{notrace,filter} files

[Steven Rostedt]

On Wed, 20 Aug 2025 16:34:46 +0800
Tengda Wu <wutengda@huaweicloud.com> wrote:

> Agreed. That is a much cleaner solution. I just tested this code and it
> worked perfectly. Looking forward to getting it into the mainline soon.

Thanks, I'll also add a Tested-by tag for you too then.

-- Steve