debugfs: fix UAF and double-free in debugfs_str read/write

debugfs_write_file_str() stores the new string via rcu_assign_pointer() and frees the old one after synchronize_rcu(). However, debugfs_read_file_str() dereferences file->private_data without holding an RCU read-side critical section, so synchronize_rcu() in a concurrent writer can return—and kfree() the old pointer—before the reader finishes using it (use-after-free).

Additionally, two concurrent writers both read the same old pointer before either calls rcu_assign_pointer(). When both subsequently call kfree(old) a double-free results.

Fix both issues:

- Read path: wrap the pointer dereference and string copy inside rcu_read_lock()/rcu_read_unlock(), using rcu_dereference() and GFP_ATOMIC to keep the allocation within the critical section.

- Write path: serialize concurrent writers with inode_lock(), and use rcu_dereference_protected() to read old under the held lock. The lock is released before synchronize_rcu()+kfree() so readers are not blocked during the grace period.

---
 fs/debugfs/file.c | 50 ++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

@@ -1018,34 +1018,32 @@
 			      size_t count, loff_t *ppos)
 {
 	struct dentry *dentry = F_DENTRY(file);
-	char *str, *copy = NULL;
-	int copy_len, len;
+	char *str, *copy;
+	int len;
 	ssize_t ret;
 
 	ret = debugfs_file_get(dentry);
 	if (unlikely(ret))
 		return ret;
 
-	str = *(char **)file->private_data;
+	/*
+	 * Pair with rcu_assign_pointer() in debugfs_write_file_str().
+	 * Without this critical section, synchronize_rcu() in a concurrent
+	 * writer can complete and free the string before we finish reading it.
+	 */
+	rcu_read_lock();
+	str = rcu_dereference(*(char __rcu **)file->private_data);
 	len = strlen(str) + 1;
-	copy = kmalloc(len, GFP_KERNEL);
-	if (!copy) {
-		debugfs_file_put(dentry);
-		return -ENOMEM;
-	}
+	copy = kmemdup(str, len, GFP_ATOMIC);
+	rcu_read_unlock();
 
-	copy_len = strscpy(copy, str, len);
 	debugfs_file_put(dentry);
-	if (copy_len < 0) {
-		kfree(copy);
-		return copy_len;
-	}
-
-	copy[copy_len] = '\n';
+	if (!copy)
+		return -ENOMEM;
 
+	copy[len - 1] = '\n';
 	ret = simple_read_from_buffer(user_buf, count, ppos, copy, len);
 	kfree(copy);
-
 	return ret;
 }
 

@@ -1061,40 +1059,48 @@
 	if (unlikely(r))
 		return r;
 
-	old = *(char **)file->private_data;
+	/*
+	 * Serialize concurrent writers: without this lock two simultaneous
+	 * calls can both read the same 'old' pointer and double-free it.
+	 */
+	inode_lock(file_inode(file));
+	old = rcu_dereference_protected(*(char __rcu **)file->private_data,
+					lockdep_is_held(&file_inode(file)->i_rwsem));
 
 	/* only allow strict concatenation */
 	r = -EINVAL;
 	if (pos && pos != strlen(old))
-		goto error;
+		goto unlock;
 
 	r = -E2BIG;
 	if (pos + count + 1 > PAGE_SIZE)
-		goto error;
+		goto unlock;
 
 	r = -ENOMEM;
 	new = kmalloc(pos + count + 1, GFP_KERNEL);
 	if (!new)
-		goto error;
+		goto unlock;
 
 	if (pos)
 		memcpy(new, old, pos);
 
 	r = -EFAULT;
 	if (copy_from_user(new + pos, user_buf, count))
-		goto error;
+		goto unlock;
 
 	new[pos + count] = '\0';
 	strim(new);
 
 	rcu_assign_pointer(*(char __rcu **)file->private_data, new);
+	inode_unlock(file_inode(file));
 	synchronize_rcu();
 	kfree(old);
 
 	debugfs_file_put(dentry);
 	return count;
 
-error:
+unlock:
+	inode_unlock(file_inode(file));
 	kfree(new);
 	debugfs_file_put(dentry);
 	return r;

2.43.0

Did you forget an Assisted-by: tag here?

And how was this found and tested?

And what kernel was tested, debugfs strings were changed a bunch in 7.1, what bug report are you working off of here?

and shouldn't this be multiple patches as you are doing two different things?

thanks,

greg k-h

Yes, apologies. Both patches now carry:

sashiko.dev flagged that the write path uses rcu_assign_pointer() / synchronize_rcu() / kfree() but the read path dereferences the same pointer without rcu_read_lock(). I then manually reviewed the write path and found the second issue: no mutual exclusion between concurrent writers, leading to a double-free of old.

Tested on QEMU/KVM with CONFIG_KASAN=y. The double-free in the write path reproduces immediately (~3900 KASAN reports in a single run with 8 concurrent writers):

  BUG: KASAN: double-free in debugfs_write_file_str+0x194/0x200

Full splat is included in patch 2/2.

The read-path UAF has a narrower race window (just a couple of instructions between pointer load and strlen()) and did not trigger in KASAN testing, but it is structurally obvious: the write path calls synchronize_rcu() + kfree(old) and the read path does not hold rcu_read_lock().

Verified against torvalds/linux master at v7.3-rc4 (commit 6812ce4e4379, 2026-09-26). Both bugs are present in the current code at debugfs_read_file_str() and debugfs_write_file_str() in fs/debugfs/file.c.

This is not from a bug report. It was found by sashiko.dev reviewing an adjacent patch and I verified it independently.

Agreed. Split into two patches below.

Aldo Ariel Panzardo (2):
debugfs: fix use-after-free in debugfs_read_file_str()
debugfs: serialize concurrent writers in debugfs_write_file_str()

 fs/debugfs/file.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

--
2.43.0

No LLM was used to generate the patch?

Again, no LLM for all of this?

I'd like to see the userspace test scripts for this...

thanks,

greg k-h

I did use Claude Code during the process, but I want to be precise
about how. It did not find the bug, produce the analysis, or generate
the patch for me. sashiko.dev originally pointed out the missing RCU protection on the read side. I then manually traced the pointer
lifetime and the write path, where I found the concurrent-writer double-free. I used Claude Code only as an additional reviewer for spelling and grammar, minor rewording, formatting, and as a sanity check for obvious mistakes.
I wrote the patch and changelog myself, and the technical analysis, implementation, KASAN testing, reproducer, and verification were all done by me. Given that limited use, would you still prefer that I add an Assisted-by tag for the LLM in the next revision? I want to make sure I disclose the tooling correctly without attributing technical work that it did not actually contribute.

My background is security research -- I spend most of my time auditing code for memory safety issues and race conditions, among other things, which is how I ended up looking at this code after sashiko flagged the missing RCU protection.

Sure. Below is the reproducer I used.

Result without fix: ~3900 "BUG: KASAN: double-free in debugfs_write_file_str" on 7.3-rc4.
Result with fix: 0 reports.

In-tree callers of debugfs_create_str() with writable files (vulnerable to the double-free):

  drivers/interconnect/debugfs-client.c:165  src_node (0600)
  drivers/interconnect/debugfs-client.c:166  dst_node (0600)
  drivers/soundwire/debugfs.c:361            firmware_file (0200)

Read-only callers (drivers/opp, sound/soc/sof, arm_scmi, i915) are exposed to the read-path UAF but not the double-free.

== debugfs_race.c (kernel module) ==

// SPDX-License-Identifier: GPL-2.0
#include <linux/module.h>
#include <linux/debugfs.h>
#include <linux/slab.h>

static struct dentry *dir;
static char *test_str;

static int __init race_init(void)

{
	test_str = kstrdup("initial_value_1234567890", GFP_KERNEL);
	if (!test_str)
		return -ENOMEM;
	dir = debugfs_create_dir("str_race", NULL);
	debugfs_create_str("test", 0666, dir, &test_str);
	pr_info("debugfs_race: /sys/kernel/debug/str_race/test created\n");
	return 0;
}

static void __exit race_exit(void)

{
	debugfs_remove_recursive(dir);
	kfree(test_str);
}

module_init(race_init);
module_exit(race_exit);
MODULE_LICENSE("GPL");

== poc.c (userspace reproducer, gcc -O2 -pthread -o poc poc.c) ==

#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <pthread.h>
#include <sched.h>

#define PATH "/sys/kernel/debug/str_race/test"
#define ITERS 5000

static volatile int go;

static void *reader_fn(void *arg)

{
	char buf[512];
	int fd = open(PATH, O_RDONLY);
	if (fd < 0) return NULL;
	while (!go) sched_yield();
	for (int i = 0; i < ITERS; i++) {
		lseek(fd, 0, SEEK_SET);
		read(fd, buf, sizeof(buf));
	}
	close(fd);
	return NULL;
}

static void *writer_fn(void *arg)

{
	int fd = open(PATH, O_WRONLY);
	if (fd < 0) return NULL;
	while (!go) sched_yield();
	for (int i = 0; i < ITERS; i++) {
		lseek(fd, 0, SEEK_SET);
		write(fd, "AAAAAAAAAAAAAAAA", 16);
	}
	close(fd);
	return NULL;
}

int main(void)

{
	pthread_t t[16];
	int i, n;

	if (access(PATH, F_OK) != 0) {
		fprintf(stderr, "Load debugfs_race.ko first.\n");
		return 1;
	}

	/* readers vs writers */
	go = 0; n = 0;
	for (i = 0; i < 4; i++) pthread_create(&t[n++], NULL, reader_fn, NULL);
	for (i = 0; i < 4; i++) pthread_create(&t[n++], NULL, writer_fn, NULL);
	go = 1;
	for (i = 0; i < n; i++) pthread_join(t[i], NULL);

	/* writers vs writers */
	go = 0; n = 0;
	for (i = 0; i < 8; i++) pthread_create(&t[n++], NULL, writer_fn, NULL);
	go = 1;
	for (i = 0; i < n; i++) pthread_join(t[i], NULL);

	printf("Done. Check: dmesg | grep KASAN\n");
	return 0;
}

== Makefile ==

KDIR ?= /lib/modules/$(shell uname -r)/build
obj-m += debugfs_race.o
all: modules poc
modules:

	$(MAKE) -C $(KDIR) M=$(CURDIR) modules
poc: poc.c
	gcc -O2 -pthread -o poc poc.c
clean:
	$(MAKE) -C $(KDIR) M=$(CURDIR) clean
	rm -f poc

thanks,
Aldo

debugfs_write_file_str() publishes a new string pointer via rcu_assign_pointer(), waits for a grace period with synchronize_rcu(), then frees the old string.

debugfs_read_file_str() dereferences file->private_data without holding an RCU read-side critical section: it loads the pointer, calls strlen() on it, and then copies the string. If a concurrent writer completes synchronize_rcu() and kfree()s the old string between the load and the use, the reader accesses freed memory.

Fix by wrapping the pointer dereference and copy inside rcu_read_lock()/rcu_read_unlock(), using rcu_dereference() to annotate the load. Switch to kmemdup() with GFP_ATOMIC so the allocation stays within the RCU critical section.

Tested on QEMU/KVM (7.3.0-rc4 + KASAN). While KASAN did not independently confirm this particular race window (it is a very narrow reader-side window), the companion double-free in the write path confirms that the RCU synchronization in this file is incomplete (see patch 2/2).

---
 fs/debugfs/file.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

index 1b3ace467777..a4b7a5cb5e01 100644
@@ -1019,30 +1019,24 @@ ssize_t debugfs_read_file_str(struct file *file, char __user *user_buf,
 {
 	struct dentry *dentry = F_DENTRY(file);
-	char *str, *copy = NULL;
-	int copy_len, len;
+	char *str, *copy;
+	int len;
 	ssize_t ret;
 
 	ret = debugfs_file_get(dentry);
 	if (unlikely(ret))
 		return ret;
 
-	str = *(char **)file->private_data;
+	rcu_read_lock();
+	str = rcu_dereference(*(char __rcu **)file->private_data);
 	len = strlen(str) + 1;
-	copy = kmalloc(len, GFP_KERNEL);
-	if (!copy) {
-		debugfs_file_put(dentry);
-		return -ENOMEM;
-	}
-
-	copy_len = strscpy(copy, str, len);
-	debugfs_file_put(dentry);
-	if (copy_len < 0) {
-		kfree(copy);
-		return copy_len;
-	}
+	copy = kmemdup(str, len, GFP_ATOMIC);
+	rcu_read_unlock();
 
-	copy[copy_len] = '\n';
+	debugfs_file_put(dentry);
+	if (!copy)
+		return -ENOMEM;
 
+	copy[len - 1] = '\n';
 	ret = simple_read_from_buffer(user_buf, count, ppos, copy, len);
 	kfree(copy);
-

return ret;
}
--
2.43.0

debugfs_write_file_str() reads the current string pointer into old, builds a replacement, publishes it with rcu_assign_pointer(), then frees old after synchronize_rcu(). There is no mutual exclusion between concurrent writers: two simultaneous calls can both load the same old pointer before either publishes a replacement, and when both later call kfree(old), the result is a double-free.

Serialize writers with inode_lock(). Use rcu_dereference_protected() to load old under the held lock, satisfying lockdep annotation. Release the lock before synchronize_rcu() + kfree() so RCU readers are not stalled during the grace period.

Tested on QEMU/KVM (7.3.0-rc4 + KASAN). A reproducer that spawns concurrent write() calls on a debugfs string file triggers the double-free reliably (~3900 KASAN reports in a single run):

  BUG: KASAN: double-free in debugfs_write_file_str+0x194/0x200
  Free of addr ffff888104542ec0 by task poc/305

  Call Trace:
   kasan_report_invalid_free+0xb8/0xe0
   check_slab_allocation+0xd9/0x100
   kfree+0x163/0x510
   debugfs_write_file_str+0x194/0x200
   vfs_write+0x1ed/0x920
   ksys_write+0xe5/0x1a0

  Allocated by task 307:
   __kmalloc_noprof+0x264/0x680
   debugfs_write_file_str+0x106/0x200

  Freed by task 306:
   kfree+0x2e5/0x510
   debugfs_write_file_str+0x194/0x200

---
 fs/debugfs/file.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index a4b7a5cb5e01..f8e2d3c9b712 100644
@@ -1060,38 +1060,42 @@ static ssize_t debugfs_write_file_str(struct file *file, const char __user *user
 	if (unlikely(r))
 		return r;
 
-	old = *(char **)file->private_data;
+	inode_lock(file_inode(file));
+	old = rcu_dereference_protected(*(char __rcu **)file->private_data,
+					lockdep_is_held(&file_inode(file)->i_rwsem));
 
 	/* only allow strict concatenation */
 	r = -EINVAL;
 	if (pos && pos != strlen(old))
-		goto error;
+		goto unlock;
 
 	r = -E2BIG;
 	if (pos + count + 1 > PAGE_SIZE)
-		goto error;
+		goto unlock;
 
 	r = -ENOMEM;
 	new = kmalloc(pos + count + 1, GFP_KERNEL);
 	if (!new)
-		goto error;
+		goto unlock;
 
 	if (pos)
 		memcpy(new, old, pos);
 
 	r = -EFAULT;
 	if (copy_from_user(new + pos, user_buf, count))
-		goto error;
+		goto unlock;
 
 	new[pos + count] = '\0';
 	strim(new);
 
 	rcu_assign_pointer(*(char __rcu **)file->private_data, new);
+	inode_unlock(file_inode(file));
 	synchronize_rcu();
 	kfree(old);
 
 	debugfs_file_put(dentry);
 	return count;
 
-error:
+unlock:
+	inode_unlock(file_inode(file));
 	kfree(new);

debugfs_file_put(dentry);
return r;
--
2.43.0

v3: regenerate patches with git format-patch against current mainline
(v2 Failed to apply due to an incorrect manual edit.). No code changes from v2.

v2: split into two patches, add Assisted-by tag, include KASAN splat.
https://lore.kernel.org/driver-core/

v1: https://lore.kernel.org/driver-core/[email protected]/

Aldo Ariel Panzardo (2):
debugfs: fix use-after-free in debugfs_read_file_str()
debugfs: serialize concurrent writers in debugfs_write_file_str()

 fs/debugfs/file.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

--
2.43.0

debugfs_write_file_str() publishes a new string pointer via rcu_assign_pointer(), waits for a grace period with synchronize_rcu(), then frees the old string.

debugfs_read_file_str() dereferences file->private_data without holding an RCU read-side critical section: it loads the pointer, calls strlen() on it, and then copies the string. If a concurrent writer completes synchronize_rcu() and kfree()s the old string between the load and the use, the reader accesses freed memory.

Fix by wrapping the pointer dereference and copy inside rcu_read_lock()/rcu_read_unlock(), using rcu_dereference() to annotate the load. Switch to kmemdup() with GFP_ATOMIC so the allocation stays within the RCU critical section.

Tested on QEMU/KVM (7.3.0-rc4 + KASAN). While KASAN did not independently confirm this particular race window (it is a very narrow reader-side window), the companion double-free in the write path confirms that the RCU synchronization in this file is incomplete (see the following patch).

---
 fs/debugfs/file.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

index 08de6652a..ac4199e9f 100644
@@ -1018,34 +1018,27 @@ ssize_t debugfs_read_file_str(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
 {
 	struct dentry *dentry = F_DENTRY(file);
-	char *str, *copy = NULL;
-	int copy_len, len;
+	char *str, *copy;
+	int len;
 	ssize_t ret;
 
 	ret = debugfs_file_get(dentry);
 	if (unlikely(ret))
 		return ret;
 
-	str = *(char **)file->private_data;
+	rcu_read_lock();
+	str = rcu_dereference(*(char __rcu **)file->private_data);
 	len = strlen(str) + 1;
-	copy = kmalloc(len, GFP_KERNEL);
-	if (!copy) {
-		debugfs_file_put(dentry);
-		return -ENOMEM;
-	}
+	copy = kmemdup(str, len, GFP_ATOMIC);
+	rcu_read_unlock();
 
-	copy_len = strscpy(copy, str, len);
 	debugfs_file_put(dentry);
-	if (copy_len < 0) {
-		kfree(copy);
-		return copy_len;
-	}
-
-	copy[copy_len] = '\n';
+	if (!copy)
+		return -ENOMEM;
 
+	copy[len - 1] = '\n';
 	ret = simple_read_from_buffer(user_buf, count, ppos, copy, len);
 	kfree(copy);
-
 	return ret;
 }
 

2.43.0

I'd like to avoid GFP_ATOMIC for this, as this doesn't seem to justify accessing memory reserves.

There are a couple of alternatives for RCU:

(1) Measure under RCU, allocate, re-check and copy under RCU, or grow the buffer and retry.

(2) Use GFP_NOWAIT and just fail under memory pressure.

(3) Allocate what userspace wants to read, not what the actual size of the string is, i.e. min(count, PAGE_SIZE).

For this specific issue I'd probably go with (1).

However, I think that the whole debugfs_create_str() API is a bit of a footgun to begin with as it implicitly imposes the same RCU constraints on the caller of debugfs_create_str() (which also incentivises GFP_ATOMIC usage because it is convinient).

For instance, icc_get_set() [1] has to do the same RCU dance and uses GFP_ATOMIC.

Other users, such as soundwire, just get it wrong and just pass the unprotected pointer to e.g. request_firmware() [2], which is a potential UAF.

So, I think this API lacks a proper synchronization contract; debugfs_create_str() doesn't mention anything at all, which may be why soundwire got it wrong.

I suggest to add a new type, e.g.

	struct debugfs_string {
		char __rcu *value;
	}

or if we want to just use a mutex for synchronization (which simplifies the allocation issue) we can just make it:

	struct debugfs_string {
		char *value;
		struct mutex lock;
	}

And then we can have helpers, such as

	int debugfs_string_set(struct debugfs_string *s, const char *value);

and

	char *debugfs_string_dup(struct debugfs_string *s);

which take care of the synchronization for the caller.

Personally, I'd just go for the mutex, as it avoids the need for the allocation dance and also allows us to provide a guard if we want to access the string and avoid a duplication of the string in the first place, e.g.

	scoped_guard(debugfs_string, s)
		matches = !strcmp(debugfs_string_read_locked(s), pattern);

Thanks,
Danilo

[1] https://elixir.bootlin.com/linux/v7.2.7/source/drivers/interconnect/debugfs-client.c#L51
[2] https://elixir.bootlin.com/linux/v7.2.7/source/drivers/soundwire/debugfs.c#L268

debugfs_write_file_str() reads the current string pointer into old, builds a replacement, publishes it with rcu_assign_pointer(), then frees old after synchronize_rcu(). There is no mutual exclusion between concurrent writers: two simultaneous calls can both load the same old pointer before either publishes a replacement, and when both later call kfree(old), the result is a double-free.

Serialize writers with inode_lock(). Use rcu_dereference_protected() to load old under the held lock, satisfying lockdep annotation. Release the lock before synchronize_rcu() + kfree() so RCU readers are not stalled during the grace period.

Tested on QEMU/KVM (7.3.0-rc4 + KASAN). A reproducer that spawns concurrent write() calls on a debugfs string file triggers the double-free reliably (~3900 KASAN reports in a single run):

  BUG: KASAN: double-free in debugfs_write_file_str+0x194/0x200
  Free of addr ffff888104542ec0 by task poc/305

  Call Trace:
   kasan_report_invalid_free+0xb8/0xe0
   check_slab_allocation+0xd9/0x100
   kfree+0x163/0x510
   debugfs_write_file_str+0x194/0x200
   vfs_write+0x1ed/0x920
   ksys_write+0xe5/0x1a0

  Allocated by task 307:
   __kmalloc_noprof+0x264/0x680
   debugfs_write_file_str+0x106/0x200

  Freed by task 306:
   kfree+0x2e5/0x510
   debugfs_write_file_str+0x194/0x200

---
 fs/debugfs/file.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index ac4199e9f..c696fa44d 100644
@@ -1054,40 +1054,44 @@ static ssize_t debugfs_write_file_str(struct file *file, const char __user *user
 	if (unlikely(r))
 		return r;
 
-	old = *(char **)file->private_data;
+	inode_lock(file_inode(file));
+	old = rcu_dereference_protected(*(char __rcu **)file->private_data,
+					lockdep_is_held(&file_inode(file)->i_rwsem));
 
 	/* only allow strict concatenation */
 	r = -EINVAL;
 	if (pos && pos != strlen(old))
-		goto error;
+		goto unlock;
 
 	r = -E2BIG;
 	if (pos + count + 1 > PAGE_SIZE)
-		goto error;
+		goto unlock;
 
 	r = -ENOMEM;
 	new = kmalloc(pos + count + 1, GFP_KERNEL);
 	if (!new)
-		goto error;
+		goto unlock;
 
 	if (pos)
 		memcpy(new, old, pos);
 
 	r = -EFAULT;
 	if (copy_from_user(new + pos, user_buf, count))
-		goto error;
+		goto unlock;
 
 	new[pos + count] = '\0';
 	strim(new);
 
 	rcu_assign_pointer(*(char __rcu **)file->private_data, new);
+	inode_unlock(file_inode(file));
 	synchronize_rcu();
 	kfree(old);
 
 	debugfs_file_put(dentry);
 	return count;
 
-error:
+unlock:
+	inode_unlock(file_inode(file));
 	kfree(new);
 	debugfs_file_put(dentry);
 	return r;

2.43.0