Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/3] debugfs: disallow NULL string creation and fix callers
@ 2026-03-23  8:58 Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 1/3] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Gui-Dong Han @ 2026-03-23  8:58 UTC (permalink / raw)
  To: gregkh, rafael, dakr, vkoul, yung-chuan.liao,
	pierre-louis.bossart
  Cc: peterz, cristian.marussi, sudeep.holla, linux-sound, driver-core,
	linux-kernel, akaieurus, me, Gui-Dong Han

A recent bug report [1] highlighted a NULL pointer dereference when
reading a debugfs string file created with a NULL pointer. The community
discussed the issue and agreed that creating string nodes with NULL is
invalid and should be forbidden at creation time [2]. Since no fix was
submitted following the discussion, I have implemented the agreed
solution.

Patch 1 modifies debugfs_create_str() to reject NULL pointers, returning
early and triggering a WARN_ON to expose offending callers.

Patch 2 is a code hygiene fix. While modifying the file, I noticed the
EXPORT_SYMBOL_GPL for debugfs_create_str() was misplaced far away from
the function body. This patch moves it to the correct location.

I carefully audited existing callers across the kernel tree. Some
drivers passing NULL have already been independently identified and
fixed [3]. The remaining subsystem (soundwire) is addressed in patch 3
by initializing its string parameter to an empty string (""). The
existing code correctly and safely handles empty strings.

While auditing, I also noticed a devm_kstrdup/kfree mismatch in the 
interconnect subsystem which has been addressed in a separate patch [4].

Changes in v2:
- Link to v1: https://lore.kernel.org/driver-core/20260317185920.43387-1-hanguidong02@gmail.com/
- Patch 1 & 2: No changes.
- Patch 3: Rewritten to use kstrdup() instead of devm_kstrdup() to match
  debugfs's kfree() behavior. Moved initialization to sdw_debugfs_init()
  to correctly handle the global pointer and avoid overwriting during
  slave probe.
- Patch 4 (drm/i915): Dropped. Further analysis confirmed those specific
  pointers are not passed to debugfs_create_str() [5].

[1] https://lore.kernel.org/lkml/17647e4c.d461.19b46144a4e.Coremail.yangshiguang1011@163.com/
[2] https://lore.kernel.org/lkml/2025122221-gag-malt-75ba@gregkh/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cc27f5c6dd1
[4] https://lore.kernel.org/linux-pm/20260318024815.7655-1-hanguidong02@gmail.com/
[5] https://lore.kernel.org/driver-core/8ac4b6ef2dfbc1dce0047dee55f2df609287a3ec@intel.com/

Gui-Dong Han (3):
  debugfs: check for NULL pointer in debugfs_create_str()
  debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str()
  soundwire: debugfs: initialize firmware_file to empty string

 drivers/soundwire/debugfs.c | 9 +++++++--
 fs/debugfs/file.c           | 7 +++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/3] debugfs: check for NULL pointer in debugfs_create_str()
  2026-03-23  8:58 [PATCH v2 0/3] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
@ 2026-03-23  8:58 ` Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 2/3] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 3/3] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
  2 siblings, 0 replies; 4+ messages in thread
From: Gui-Dong Han @ 2026-03-23  8:58 UTC (permalink / raw)
  To: gregkh, rafael, dakr, vkoul, yung-chuan.liao,
	pierre-louis.bossart
  Cc: peterz, cristian.marussi, sudeep.holla, linux-sound, driver-core,
	linux-kernel, akaieurus, me, Gui-Dong Han, yangshiguang

Passing a NULL pointer to debugfs_create_str() leads to a NULL pointer
dereference when the debugfs file is read. Following upstream
discussions, forbid the creation of debugfs string files with NULL
pointers. Add a WARN_ON() to expose offending callers and return early.

Fixes: 9af0440ec86e ("debugfs: Implement debugfs_create_str()")
Reported-by: yangshiguang <yangshiguang@xiaomi.com>
Closes: https://lore.kernel.org/lkml/2025122221-gag-malt-75ba@gregkh/
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
v2:
* No changes.
---
 fs/debugfs/file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 3376ab6a519d..a941d73251b0 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -1127,7 +1127,7 @@ static const struct file_operations fops_str_wo = {
  *          directory dentry if set.  If this parameter is %NULL, then the
  *          file will be created in the root of the debugfs filesystem.
  * @value: a pointer to the variable that the file should read to and write
- *         from.
+ *         from. This pointer and the string it points to must not be %NULL.
  *
  * This function creates a file in debugfs with the given name that
  * contains the value of the variable @value.  If the @mode variable is so
@@ -1136,6 +1136,9 @@ static const struct file_operations fops_str_wo = {
 void debugfs_create_str(const char *name, umode_t mode,
 			struct dentry *parent, char **value)
 {
+	if (WARN_ON(!value || !*value))
+		return;
+
 	debugfs_create_mode_unsafe(name, mode, parent, value, &fops_str,
 				   &fops_str_ro, &fops_str_wo);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/3] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str()
  2026-03-23  8:58 [PATCH v2 0/3] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 1/3] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
@ 2026-03-23  8:58 ` Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 3/3] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
  2 siblings, 0 replies; 4+ messages in thread
From: Gui-Dong Han @ 2026-03-23  8:58 UTC (permalink / raw)
  To: gregkh, rafael, dakr, vkoul, yung-chuan.liao,
	pierre-louis.bossart
  Cc: peterz, cristian.marussi, sudeep.holla, linux-sound, driver-core,
	linux-kernel, akaieurus, me, Gui-Dong Han

The EXPORT_SYMBOL_GPL() for debugfs_create_str was placed incorrectly
away from the function definition. Move it immediately below the
debugfs_create_str() function where it belongs.

Fixes: d60b59b96795 ("debugfs: Export debugfs_create_str symbol")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
v2:
* No changes.
---
 fs/debugfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index a941d73251b0..edd6aafbfbaa 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -1047,7 +1047,6 @@ ssize_t debugfs_read_file_str(struct file *file, char __user *user_buf,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(debugfs_create_str);
 
 static ssize_t debugfs_write_file_str(struct file *file, const char __user *user_buf,
 				      size_t count, loff_t *ppos)
@@ -1142,6 +1141,7 @@ void debugfs_create_str(const char *name, umode_t mode,
 	debugfs_create_mode_unsafe(name, mode, parent, value, &fops_str,
 				   &fops_str_ro, &fops_str_wo);
 }
+EXPORT_SYMBOL_GPL(debugfs_create_str);
 
 static ssize_t read_file_blob(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 3/3] soundwire: debugfs: initialize firmware_file to empty string
  2026-03-23  8:58 [PATCH v2 0/3] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 1/3] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
  2026-03-23  8:58 ` [PATCH v2 2/3] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
@ 2026-03-23  8:58 ` Gui-Dong Han
  2 siblings, 0 replies; 4+ messages in thread
From: Gui-Dong Han @ 2026-03-23  8:58 UTC (permalink / raw)
  To: gregkh, rafael, dakr, vkoul, yung-chuan.liao,
	pierre-louis.bossart
  Cc: peterz, cristian.marussi, sudeep.holla, linux-sound, driver-core,
	linux-kernel, akaieurus, me, Gui-Dong Han, yangshiguang

Passing NULL to debugfs_create_str() causes a NULL pointer dereference,
and creating debugfs nodes with NULL string pointers is no longer
permitted.

Additionally, firmware_file is a global pointer. Previously, adding every
new slave blindly overwrote it with NULL.

Fix these issues by initializing firmware_file to an allocated empty
string once in the subsystem init path (sdw_debugfs_init), and freeing
it in the exit path. Existing driver code handles empty strings
correctly.

Fixes: fe46d2a4301d ("soundwire: debugfs: add interface to read/write commands")
Reported-by: yangshiguang <yangshiguang@xiaomi.com>
Closes: https://lore.kernel.org/lkml/17647e4c.d461.19b46144a4e.Coremail.yangshiguang1011@163.com/
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
@SoundWire maintainers: Reviewed-by and Acked-by tags are welcome. Based
on my testing, reading a string node created with a NULL pointer causes
a crash, and writing to it returns -EINVAL. This completely breaks the
interface, making me highly suspect this code has never actually been
used. Additionally, sharing the global firmware_file pointer is
inherently racy. I will investigate fixing or removing it entirely in a
follow-up patch, as it falls outside the scope of this series.

v2:
* Replace devm_kstrdup() with kstrdup() to fix allocation/free mismatch
with debugfs.
* Move initialization to sdw_debugfs_init() to correctly handle the global
pointer and avoid overwriting during slave probe.
---
 drivers/soundwire/debugfs.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
index ccc9670ef77c..2905ec19b838 100644
--- a/drivers/soundwire/debugfs.c
+++ b/drivers/soundwire/debugfs.c
@@ -358,8 +358,8 @@ void sdw_slave_debugfs_init(struct sdw_slave *slave)
 	debugfs_create_file("go", 0200, d, slave, &cmd_go_fops);
 
 	debugfs_create_file("read_buffer", 0400, d, slave, &read_buffer_fops);
-	firmware_file = NULL;
-	debugfs_create_str("firmware_file", 0200, d, &firmware_file);
+	if (firmware_file)
+		debugfs_create_str("firmware_file", 0200, d, &firmware_file);
 
 	slave->debugfs = d;
 }
@@ -371,10 +371,15 @@ void sdw_slave_debugfs_exit(struct sdw_slave *slave)
 
 void sdw_debugfs_init(void)
 {
+	if (!firmware_file)
+		firmware_file = kstrdup("", GFP_KERNEL);
+
 	sdw_debugfs_root = debugfs_create_dir("soundwire", NULL);
 }
 
 void sdw_debugfs_exit(void)
 {
 	debugfs_remove_recursive(sdw_debugfs_root);
+	kfree(firmware_file);
+	firmware_file = NULL;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-23  9:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23  8:58 [PATCH v2 0/3] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
2026-03-23  8:58 ` [PATCH v2 1/3] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
2026-03-23  8:58 ` [PATCH v2 2/3] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
2026-03-23  8:58 ` [PATCH v2 3/3] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox