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