Re: [PATCH 4/4] drm/i915/display: initialize string params to empty strings

[Jani Nikula]

On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@xxxxxxxxx> wrote:
> On Wed, Mar 18, 2026 at 4:12 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>>
>> On Wed, 18 Mar 2026, Gui-Dong Han <hanguidong02@xxxxxxxxx> 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@xxxxxxxxx>
>> > ---
>> >  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