* [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* 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
* [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 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