On Wed, Mar 18, 2026 at 4:32 PM Jani Nikula <[email protected]> wrote: > > On Wed, 18 Mar 2026, Gui-Dong Han <[email protected]> wrote: > > On Wed, Mar 18, 2026 at 4:12 AM Jani Nikula <[email protected]> > > wrote: > >> > >> On Wed, 18 Mar 2026, Gui-Dong Han <[email protected]> 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 <[email protected]> > >> > --- > >> > 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.
