public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/4] debugfs: disallow NULL string creation and fix callers
@ 2026-03-17 18:59 Gui-Dong Han
  2026-03-17 18:59 ` [PATCH 1/4] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-17 18:59 UTC (permalink / raw)
  To: gregkh, dakr, rafael
  Cc: linux-kernel, driver-core, intel-gfx, intel-xe, dri-devel,
	linux-sound, 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 two subsystems (soundwire and drm/i915) are
addressed in patches 3 and 4 by initializing their respective string
parameters to empty strings (""). The existing logic in both subsystems
correctly and safely handles empty strings.

[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

Gui-Dong Han (4):
  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
  drm/i915/display: initialize string params to empty strings

 drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
 drivers/soundwire/debugfs.c                         | 5 +++--
 fs/debugfs/file.c                                   | 7 +++++--
 3 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.43.0


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

* [PATCH 1/4] debugfs: check for NULL pointer in debugfs_create_str()
  2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
@ 2026-03-17 18:59 ` Gui-Dong Han
  2026-03-17 18:59 ` [PATCH 2/4] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-17 18:59 UTC (permalink / raw)
  To: gregkh, dakr, rafael
  Cc: linux-kernel, driver-core, intel-gfx, intel-xe, dri-devel,
	linux-sound, 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>
---
 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] 11+ messages in thread

* [PATCH 2/4] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str()
  2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
  2026-03-17 18:59 ` [PATCH 1/4] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
@ 2026-03-17 18:59 ` Gui-Dong Han
  2026-03-17 19:10 ` [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-17 18:59 UTC (permalink / raw)
  To: gregkh, dakr, rafael
  Cc: linux-kernel, driver-core, intel-gfx, intel-xe, dri-devel,
	linux-sound, 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>
---
 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] 11+ messages in thread

* [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string
  2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
  2026-03-17 18:59 ` [PATCH 1/4] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
  2026-03-17 18:59 ` [PATCH 2/4] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
@ 2026-03-17 19:10 ` Gui-Dong Han
  2026-03-18  4:14   ` Gui-Dong Han
  2026-03-17 19:15 ` [PATCH 4/4] drm/i915/display: initialize string params to empty strings Gui-Dong Han
  2026-03-23 18:15 ` ✗ LGCI.VerificationFailed: failure for debugfs: disallow NULL string creation and fix callers Patchwork
  4 siblings, 1 reply; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-17 19:10 UTC (permalink / raw)
  To: gregkh, dakr, rafael, vkoul, yung-chuan.liao
  Cc: pierre-louis.bossart, rander.wang, linux-kernel, driver-core,
	intel-gfx, intel-xe, dri-devel, linux-sound, akaieurus, me,
	Gui-Dong Han, yangshiguang

Passing NULL to debugfs_create_str() causes a NULL pointer dereference
upon reading, and creating debugfs nodes with NULL string pointers is no
longer permitted. Change the initialization of firmware_file to an
allocated empty string. Existing driver code using this field 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>
---
 drivers/soundwire/debugfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
index ccc9670ef77c..d4abe8bfca76 100644
--- a/drivers/soundwire/debugfs.c
+++ b/drivers/soundwire/debugfs.c
@@ -358,8 +358,9 @@ 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);
+	firmware_file = devm_kstrdup(&slave->dev, "", GFP_KERNEL);
+	if (firmware_file)
+		debugfs_create_str("firmware_file", 0200, d, &firmware_file);
 
 	slave->debugfs = d;
 }
-- 
2.43.0


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

* [PATCH 4/4] drm/i915/display: initialize string params to empty strings
  2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
                   ` (2 preceding siblings ...)
  2026-03-17 19:10 ` [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
@ 2026-03-17 19:15 ` Gui-Dong Han
  2026-03-17 20:12   ` Jani Nikula
  2026-03-23 18:15 ` ✗ LGCI.VerificationFailed: failure for debugfs: disallow NULL string creation and fix callers Patchwork
  4 siblings, 1 reply; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-17 19:15 UTC (permalink / raw)
  To: gregkh, dakr, rafael, jani.nikula, rodrigo.vivi, joonas.lahtinen,
	tursulin, airlied, simona
  Cc: gustavo.sousa, demarchi, jouni.hogander, luciano.coelho,
	linux-kernel, driver-core, intel-gfx, intel-xe, dri-devel,
	linux-sound, akaieurus, me, Gui-Dong Han

Passing NULL to debugfs_create_str() causes a NULL pointer dereference
upon reading, and is no longer permitted. Change the default values of
dmc_firmware_path and vbt_firmware to empty strings ("").

Existing code that consumes these parameters already verifies both
pointer validity and string length, so empty strings are handled
correctly. Furthermore, heap allocation is not required here: these
debugfs parameters are created with strictly read-only permissions
(0400). As a result, the debugfs write operation is never invoked,
meaning the static empty string will not be erroneously freed by
kfree().

Fixes: e9913f0bd2e1 ("drm/i915/display: move dmc_firmware_path to display params")
Fixes: 29292bc6cc37 ("drm/i915/display: Move vbt_firmware module parameter under display")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
index b95ecf728daa..0a8cad98d480 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.h
+++ b/drivers/gpu/drm/i915/display/intel_display_params.h
@@ -23,8 +23,8 @@ struct drm_printer;
  *       debugfs file
  */
 #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
-	param(char *, dmc_firmware_path, NULL, 0400) \
-	param(char *, vbt_firmware, NULL, 0400) \
+	param(char *, dmc_firmware_path, "", 0400) \
+	param(char *, vbt_firmware, "", 0400) \
 	param(int, lvds_channel_mode, 0, 0400) \
 	param(int, panel_use_ssc, -1, 0600) \
 	param(int, vbt_sdvo_panel_type, -1, 0400) \
-- 
2.43.0


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

* Re: [PATCH 4/4] drm/i915/display: initialize string params to empty strings
  2026-03-17 19:15 ` [PATCH 4/4] drm/i915/display: initialize string params to empty strings Gui-Dong Han
@ 2026-03-17 20:12   ` Jani Nikula
  2026-03-18  1:53     ` Gui-Dong Han
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2026-03-17 20:12 UTC (permalink / raw)
  To: Gui-Dong Han, gregkh, dakr, rafael, rodrigo.vivi, joonas.lahtinen,
	tursulin, airlied, simona
  Cc: gustavo.sousa, demarchi, jouni.hogander, luciano.coelho,
	linux-kernel, driver-core, intel-gfx, intel-xe, dri-devel,
	linux-sound, akaieurus, me, Gui-Dong Han

On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
> Passing NULL to debugfs_create_str() causes a NULL pointer dereference
> upon reading, and is no longer permitted. Change the default values of
> dmc_firmware_path and vbt_firmware to empty strings ("").
>
> Existing code that consumes these parameters already verifies both
> pointer validity and string length, so empty strings are handled
> correctly. Furthermore, heap allocation is not required here: these
> debugfs parameters are created with strictly read-only permissions
> (0400). As a result, the debugfs write operation is never invoked,
> meaning the static empty string will not be erroneously freed by
> kfree().
>
> Fixes: e9913f0bd2e1 ("drm/i915/display: move dmc_firmware_path to display params")
> Fixes: 29292bc6cc37 ("drm/i915/display: Move vbt_firmware module parameter under display")
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
> index b95ecf728daa..0a8cad98d480 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> @@ -23,8 +23,8 @@ struct drm_printer;
>   *       debugfs file
>   */
>  #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
> -	param(char *, dmc_firmware_path, NULL, 0400) \
> -	param(char *, vbt_firmware, NULL, 0400) \
> +	param(char *, dmc_firmware_path, "", 0400) \
> +	param(char *, vbt_firmware, "", 0400) \

Admittedly this is all very convoluted, but these NULL pointers (or
pointers to them) are never passed to debugfs_create_str().

BR,
Jani.


>  	param(int, lvds_channel_mode, 0, 0400) \
>  	param(int, panel_use_ssc, -1, 0600) \
>  	param(int, vbt_sdvo_panel_type, -1, 0400) \

-- 
Jani Nikula, Intel

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

* Re: [PATCH 4/4] drm/i915/display: initialize string params to empty strings
  2026-03-17 20:12   ` Jani Nikula
@ 2026-03-18  1:53     ` Gui-Dong Han
  2026-03-18  8:32       ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-18  1:53 UTC (permalink / raw)
  To: Jani Nikula
  Cc: gregkh, dakr, rafael, rodrigo.vivi, joonas.lahtinen, tursulin,
	airlied, simona, gustavo.sousa, demarchi, jouni.hogander,
	luciano.coelho, linux-kernel, driver-core, intel-gfx, intel-xe,
	dri-devel, linux-sound, akaieurus, me

On Wed, Mar 18, 2026 at 4:12 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
> > Passing NULL to debugfs_create_str() causes a NULL pointer dereference
> > upon reading, and is no longer permitted. Change the default values of
> > dmc_firmware_path and vbt_firmware to empty strings ("").
> >
> > Existing code that consumes these parameters already verifies both
> > pointer validity and string length, so empty strings are handled
> > correctly. Furthermore, heap allocation is not required here: these
> > debugfs parameters are created with strictly read-only permissions
> > (0400). As a result, the debugfs write operation is never invoked,
> > meaning the static empty string will not be erroneously freed by
> > kfree().
> >
> > Fixes: e9913f0bd2e1 ("drm/i915/display: move dmc_firmware_path to display params")
> > Fixes: 29292bc6cc37 ("drm/i915/display: Move vbt_firmware module parameter under display")
> > Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
> > index b95ecf728daa..0a8cad98d480 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> > @@ -23,8 +23,8 @@ struct drm_printer;
> >   *       debugfs file
> >   */
> >  #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
> > -     param(char *, dmc_firmware_path, NULL, 0400) \
> > -     param(char *, vbt_firmware, NULL, 0400) \
> > +     param(char *, dmc_firmware_path, "", 0400) \
> > +     param(char *, vbt_firmware, "", 0400) \
>
> Admittedly this is all very convoluted, but these NULL pointers (or
> pointers to them) are never passed to debugfs_create_str().

Hi Jani,

Thanks for your review.

Could you elaborate on why they are never passed? Looking at
intel_display_debugfs_params.c, the intel_display_debugfs_params()
function iterates over INTEL_DISPLAY_PARAMS_FOR_EACH using the
REGISTER macro. This eventually calls
_intel_display_param_create_file(), which uses _Generic to dispatch
char ** types to debugfs_create_str().

Thanks.

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

* Re: [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string
  2026-03-17 19:10 ` [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
@ 2026-03-18  4:14   ` Gui-Dong Han
  0 siblings, 0 replies; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-18  4:14 UTC (permalink / raw)
  To: hanguidong02
  Cc: akaieurus, dakr, dri-devel, driver-core, gregkh, intel-gfx,
	intel-xe, linux-kernel, linux-sound, me, pierre-louis.bossart,
	rafael, rander.wang, vkoul, yangshiguang, yung-chuan.liao

On Wed, Mar 18, 2026 at 3:11 AM Gui-Dong Han <hanguidong02@gmail.com> wrote:
>
> Passing NULL to debugfs_create_str() causes a NULL pointer dereference
> upon reading, and creating debugfs nodes with NULL string pointers is no
> longer permitted. Change the initialization of firmware_file to an
> allocated empty string. Existing driver code using this field 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>
> ---
>  drivers/soundwire/debugfs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
> index ccc9670ef77c..d4abe8bfca76 100644
> --- a/drivers/soundwire/debugfs.c
> +++ b/drivers/soundwire/debugfs.c
> @@ -358,8 +358,9 @@ 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);
> +       firmware_file = devm_kstrdup(&slave->dev, "", GFP_KERNEL);
> +       if (firmware_file)
> +               debugfs_create_str("firmware_file", 0200, d, &firmware_file);

I initially patterned this fix after commit 8cc27f5c6dd1 [1] by using
devm_kstrdup(). However, I realized that approach is flawed:
debugfs_write_file_str() calls a raw kfree(), which causes a mismatch.
I have submitted a separate patch [2] to fix that existing commit.

Additionally, firmware_file is a global pointer in this driver. The
original code blindly overwrote it with NULL every time a new slave was
added.

To fix both issues properly, I moved the allocation to the subsystem
init and exit paths so it is only allocated once.

The updated v2 patch is included below for review. I will wait for
further comments on the rest of the series and include this updated
patch if a full v2 series is required.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cc27f5c6dd1
[2] https://lore.kernel.org/linux-pm/20260318024815.7655-1-hanguidong02@gmail.com/

From bbaff3bc33746a965a2387ffe8302d05e700a1c3 Mon Sep 17 00:00:00 2001
From: Gui-Dong Han <hanguidong02@gmail.com>
Date: Wed, 18 Mar 2026 03:10:29 +0800
Subject: [PATCH v2 3/4] soundwire: debugfs: initialize firmware_file to empty string

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>
---
v2:
* Replace devm_kstrdup() with kstrdup() to fix allocation/free mismatch
with debugfs.
* Move initialization to subsystem init/exit paths to avoid overwriting
the global pointer on every 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] 11+ messages in thread

* Re: [PATCH 4/4] drm/i915/display: initialize string params to empty strings
  2026-03-18  1:53     ` Gui-Dong Han
@ 2026-03-18  8:32       ` Jani Nikula
  2026-03-18  8:47         ` Gui-Dong Han
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2026-03-18  8:32 UTC (permalink / raw)
  To: Gui-Dong Han
  Cc: gregkh, dakr, rafael, rodrigo.vivi, joonas.lahtinen, tursulin,
	airlied, simona, gustavo.sousa, demarchi, jouni.hogander,
	luciano.coelho, linux-kernel, driver-core, intel-gfx, intel-xe,
	dri-devel, linux-sound, akaieurus, me

On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
> On Wed, Mar 18, 2026 at 4:12 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>
>> On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
>> > Passing NULL to debugfs_create_str() causes a NULL pointer dereference
>> > upon reading, and is no longer permitted. Change the default values of
>> > dmc_firmware_path and vbt_firmware to empty strings ("").
>> >
>> > Existing code that consumes these parameters already verifies both
>> > pointer validity and string length, so empty strings are handled
>> > correctly. Furthermore, heap allocation is not required here: these
>> > debugfs parameters are created with strictly read-only permissions
>> > (0400). As a result, the debugfs write operation is never invoked,
>> > meaning the static empty string will not be erroneously freed by
>> > kfree().
>> >
>> > Fixes: e9913f0bd2e1 ("drm/i915/display: move dmc_firmware_path to display params")
>> > Fixes: 29292bc6cc37 ("drm/i915/display: Move vbt_firmware module parameter under display")
>> > Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
>> > index b95ecf728daa..0a8cad98d480 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_params.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
>> > @@ -23,8 +23,8 @@ struct drm_printer;
>> >   *       debugfs file
>> >   */
>> >  #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
>> > -     param(char *, dmc_firmware_path, NULL, 0400) \
>> > -     param(char *, vbt_firmware, NULL, 0400) \
>> > +     param(char *, dmc_firmware_path, "", 0400) \
>> > +     param(char *, vbt_firmware, "", 0400) \
>>
>> Admittedly this is all very convoluted, but these NULL pointers (or
>> pointers to them) are never passed to debugfs_create_str().
>
> Hi Jani,
>
> Thanks for your review.
>
> Could you elaborate on why they are never passed? Looking at
> intel_display_debugfs_params.c, the intel_display_debugfs_params()
> function iterates over INTEL_DISPLAY_PARAMS_FOR_EACH using the
> REGISTER macro. This eventually calls
> _intel_display_param_create_file(), which uses _Generic to dispatch
> char ** types to debugfs_create_str().

In _intel_display_param_create_file(), valp is &display->params.x, where
x is dmc_firmware_path or vbt_firmware.

display->params gets initialized using intel_display_params_copy() when
struct intel_display is allocated in intel_display_device_probe(). In
intel_display_params_copy(), _param_dup_charp() handles the NULL
initializer.

Granted, if the kstrdup() fails, you could end up having NULL there, but
at that point it's fine if your debugfs_create_str() change barfs and
bails out.

Like I said, it's convoluted. ;)

BR,
Jani.



-- 
Jani Nikula, Intel

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

* Re: [PATCH 4/4] drm/i915/display: initialize string params to empty strings
  2026-03-18  8:32       ` Jani Nikula
@ 2026-03-18  8:47         ` Gui-Dong Han
  0 siblings, 0 replies; 11+ messages in thread
From: Gui-Dong Han @ 2026-03-18  8:47 UTC (permalink / raw)
  To: Jani Nikula
  Cc: gregkh, dakr, rafael, rodrigo.vivi, joonas.lahtinen, tursulin,
	airlied, simona, gustavo.sousa, demarchi, jouni.hogander,
	luciano.coelho, linux-kernel, driver-core, intel-gfx, intel-xe,
	dri-devel, linux-sound, akaieurus, me

On Wed, Mar 18, 2026 at 4:32 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
> > On Wed, Mar 18, 2026 at 4:12 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >>
> >> On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@gmail.com> wrote:
> >> > Passing NULL to debugfs_create_str() causes a NULL pointer dereference
> >> > upon reading, and is no longer permitted. Change the default values of
> >> > dmc_firmware_path and vbt_firmware to empty strings ("").
> >> >
> >> > Existing code that consumes these parameters already verifies both
> >> > pointer validity and string length, so empty strings are handled
> >> > correctly. Furthermore, heap allocation is not required here: these
> >> > debugfs parameters are created with strictly read-only permissions
> >> > (0400). As a result, the debugfs write operation is never invoked,
> >> > meaning the static empty string will not be erroneously freed by
> >> > kfree().
> >> >
> >> > Fixes: e9913f0bd2e1 ("drm/i915/display: move dmc_firmware_path to display params")
> >> > Fixes: 29292bc6cc37 ("drm/i915/display: Move vbt_firmware module parameter under display")
> >> > Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_display_params.h | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
> >> > index b95ecf728daa..0a8cad98d480 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> >> > +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> >> > @@ -23,8 +23,8 @@ struct drm_printer;
> >> >   *       debugfs file
> >> >   */
> >> >  #define INTEL_DISPLAY_PARAMS_FOR_EACH(param) \
> >> > -     param(char *, dmc_firmware_path, NULL, 0400) \
> >> > -     param(char *, vbt_firmware, NULL, 0400) \
> >> > +     param(char *, dmc_firmware_path, "", 0400) \
> >> > +     param(char *, vbt_firmware, "", 0400) \
> >>
> >> Admittedly this is all very convoluted, but these NULL pointers (or
> >> pointers to them) are never passed to debugfs_create_str().
> >
> > Hi Jani,
> >
> > Thanks for your review.
> >
> > Could you elaborate on why they are never passed? Looking at
> > intel_display_debugfs_params.c, the intel_display_debugfs_params()
> > function iterates over INTEL_DISPLAY_PARAMS_FOR_EACH using the
> > REGISTER macro. This eventually calls
> > _intel_display_param_create_file(), which uses _Generic to dispatch
> > char ** types to debugfs_create_str().
>
> In _intel_display_param_create_file(), valp is &display->params.x, where
> x is dmc_firmware_path or vbt_firmware.
>
> display->params gets initialized using intel_display_params_copy() when
> struct intel_display is allocated in intel_display_device_probe(). In
> intel_display_params_copy(), _param_dup_charp() handles the NULL
> initializer.
>
> Granted, if the kstrdup() fails, you could end up having NULL there, but
> at that point it's fine if your debugfs_create_str() change barfs and
> bails out.
>
> Like I said, it's convoluted. ;)

Ah, that makes perfect sense.

Thanks for taking the time to explain this in detail! Then we could
drop this patch from the series.

Thanks.

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

* ✗ LGCI.VerificationFailed: failure for debugfs: disallow NULL string creation and fix callers
  2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
                   ` (3 preceding siblings ...)
  2026-03-17 19:15 ` [PATCH 4/4] drm/i915/display: initialize string params to empty strings Gui-Dong Han
@ 2026-03-23 18:15 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2026-03-23 18:15 UTC (permalink / raw)
  To: Gui-Dong Han; +Cc: intel-gfx

== Series Details ==

Series: debugfs: disallow NULL string creation and fix callers
URL   : https://patchwork.freedesktop.org/series/163711/
State : failure

== Summary ==

Address 'hanguidong02@gmail.com' is not on the allowlist, which prevents CI from being triggered for this patch.
If you want Intel GFX CI to accept this address, please contact the script maintainers at i915-ci-infra@lists.freedesktop.org.
Exception occurred during validation, bailing out!



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

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

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 18:59 [PATCH 0/4] debugfs: disallow NULL string creation and fix callers Gui-Dong Han
2026-03-17 18:59 ` [PATCH 1/4] debugfs: check for NULL pointer in debugfs_create_str() Gui-Dong Han
2026-03-17 18:59 ` [PATCH 2/4] debugfs: fix placement of EXPORT_SYMBOL_GPL for debugfs_create_str() Gui-Dong Han
2026-03-17 19:10 ` [PATCH 3/4] soundwire: debugfs: initialize firmware_file to empty string Gui-Dong Han
2026-03-18  4:14   ` Gui-Dong Han
2026-03-17 19:15 ` [PATCH 4/4] drm/i915/display: initialize string params to empty strings Gui-Dong Han
2026-03-17 20:12   ` Jani Nikula
2026-03-18  1:53     ` Gui-Dong Han
2026-03-18  8:32       ` Jani Nikula
2026-03-18  8:47         ` Gui-Dong Han
2026-03-23 18:15 ` ✗ LGCI.VerificationFailed: failure for debugfs: disallow NULL string creation and fix callers Patchwork

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