[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