Re: [PATCH] debugfs: serialize debugfs_create_str() writers

On Mon, Aug 03, 2026 at 08:19:05AM +0200, Greg KH wrote:
> Ouch, you are doing to serialize _all_ debugfs strings on one lock?
>
> This feels wrong :(
>
> Wait, why rcu if you have a lock?
>
> guard() is nicer.
>
> My larger question is, what code is broken because of this?  What
> debugfs string replacements are happening?  I hate the string debugfs
> code as it has had lots of issues like this over the years so maybe we
> should just drop it and force users to "roll their own" implementation
> that would be much simpler without the rcu/locking mess at all?

Thanks for the review.

The patch was trying to fix a double-free.  Two concurrent writes to the same
debugfs_create_str() file can both observe the same old pointer before
either replacement is published, and then both free that old string.  I
reproduced this with KASAN and got:

  BUG: KASAN: double-free in debugfs_write_file_str()

I agree with your comments that the current fix is not a good direction.  A
global mutex serializes unrelated debugfs string files, and mixing that with
the RCU read side makes the helper more complicated.

The directly writable in-tree users I found are:

  drivers/interconnect/debugfs-client.c:
    /sys/kernel/debug/interconnect/test_client/src_node, mode 0600
    /sys/kernel/debug/interconnect/test_client/dst_node, mode 0600

  drivers/soundwire/debugfs.c:
    firmware_file, mode 0200

So the write path is reachable through in-tree debugfs users, although I
do not know whether anyone relies on concurrent writes to these files in
practice.

I can rework this in a few ways:

  1. make the locking per-file instead of global, if we want to keep the
     generic writable string helper;

  2. drop the write support from debugfs_create_str() and convert the
     writable users to their own small file operations;

  3. take another direction if you have a preferred approach.

Dropping write support would avoid keeping this locking in the generic
helper, but debugfs_create_str() is exported, so that would also change
behavior for any out-of-tree users relying on writable strings.

Please let me know which direction you prefer.  If none of these options
looks worthwhile for debugfs, I am also fine with dropping this patch.

Thanks,
Yichong