Re: [PATCH v3 0/3] debugfs: make debugfs_create_str() read-only

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Oct 1, 2026 at 3:19 PM CEST, Greg KH wrote:
> On Thu, Oct 01, 2026 at 03:10:45PM +0200, Danilo Krummrich wrote:
>> On Thu Oct 1, 2026 at 2:51 PM CEST, Greg KH wrote:
>> > On Fri, Aug 07, 2026 at 06:00:50PM +0800, Yichong Chen wrote:
>> >> debugfs_create_str() has a generic write implementation that replaces the
>> >> backing string.  Concurrent writers can race and free the same old string
>> >> twice.
>> >> 
>> >> Instead of adding more locking to the generic helper, convert the existing
>> >> writable in-tree users to local file operations and make
>> >> debugfs_create_str() read-only.
>> >> 
>> >> Changes since v2:
>> >> - Use scoped mutex guards in the interconnect and SoundWire conversions.
>> >> - Clarify why GFP_KERNEL is safe in the interconnect conversion after the
>> >>   RCU read-side critical section is removed.
>> >> - Drop the unnecessary firmware_file = NULL assignment in the SoundWire
>> >>   exit path.
>> >> - Use WARN() instead of WARN_ONCE() so each writable debugfs_create_str()
>> >>   caller can be reported.
>> >
>> > Sorry for the delay, now applied.
>> 
>> This series fell through the cracks on my end. I also reported this issue in [1]
>> and I agree making debugfs_create_str() read-only is the best fix for now.
>> 
>> However, it duplicates code and I think having a proper helper as suggested in
>> [1] would be nice follow-up.
>> 
>> [1] https://lore.kernel.org/driver-core/DLPDB44JJRGJ.3K6JNS746M7QC@kernel.org/
>
> Yes, that would be nice, but for read-only debugfs strings, let's keep
> writable ones away if at all possible :)

The two converted subsystems now have identical code that can easily be
generalized. And I think we have other users that open code this too.

For instance, soundwire would collapse to just:

In sdw_debugfs_init():

	/* Initialize struct debugfs_string */
	ret = debugfs_string_create(&firmware_file, "");

In sdw_debugfs_exit():

	/* Free struct debugfs_string */
	debugfs_string_free(&firmware_file);

In sdw_slave_debugfs_init():

	debugfs_create_str("firmware_file", 0600, d, &firmware_file);

In cmd_go():

	scoped_guard(debugfs_string, &firmware_file)
		ret = request_firmware(&fw,
				       debugfs_string_read_locked(&firmware_file),
				       &slave->dev);

Thanks,
Danilo