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

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

[Aldo Ariel Panzardo]

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.

Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/debugfs/file.c | 50 ++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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

Re: [PATCH] debugfs: fix UAF and double-free in debugfs_str read/write

[Greg KH]

On Fri, Sep 25, 2026 at 02:58:27PM -0300, Aldo Ariel Panzardo wrote:
> 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.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

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

[PATCH v2 0/2] debugfs: fix UAF and double-free in debugfs_str read/write

[Aldo Ariel Panzardo]

On Sat, Sep 26, 2026 at 08:53:06AM +0200, Greg Kroah-Hartman wrote:
> Did you forget an Assisted-by: tag here?

Yes, apologies.  Both patches now carry:

  Assisted-by: sashiko.dev <sashiko-bot@kernel.org>

> And how was this found and tested?

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().

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

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.

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

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

[PATCH v2 1/2] debugfs: fix use-after-free in debugfs_read_file_str()

[Aldo Ariel Panzardo]

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).

Cc: stable@vger.kernel.org
Assisted-by: sashiko.dev <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/debugfs/file.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 1b3ace467777..a4b7a5cb5e01 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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

[PATCH v2 2/2] debugfs: serialize concurrent writers in debugfs_write_file_str()

[Aldo Ariel Panzardo]

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

Cc: stable@vger.kernel.org
Assisted-by: sashiko.dev <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/debugfs/file.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index a4b7a5cb5e01..f8e2d3c9b712 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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

[PATCH v3 0/2] debugfs: fix UAF and double-free in debugfs_str read/write

[Aldo Ariel Panzardo]

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/20260925175831.3701812-1-qwe.aldo@gmail.com/

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

[PATCH v3 1/2] debugfs: fix use-after-free in debugfs_read_file_str()

[Aldo Ariel Panzardo]

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).

Cc: stable@vger.kernel.org
Assisted-by: sashiko.dev <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/debugfs/file.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 08de6652a..ac4199e9f 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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

[PATCH v3 2/2] debugfs: serialize concurrent writers in debugfs_write_file_str()

[Aldo Ariel Panzardo]

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

Cc: stable@vger.kernel.org
Assisted-by: sashiko.dev <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/debugfs/file.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index ac4199e9f..c696fa44d 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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

Re: [PATCH v3 1/2] debugfs: fix use-after-free in debugfs_read_file_str()

[Danilo Krummrich]

On Sat Sep 26, 2026 at 4:09 PM CEST, Aldo Ariel Panzardo wrote:
> Switch to kmemdup() with GFP_ATOMIC so the allocation stays within the RCU
> critical section.

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