[PATCH v2] drm/amd/display: Fix NULL pointer dereference in dmub_tracebuffer_show

[Li, Roman]

[Public]

Reviewed-by: Roman Li <roman.li at amd.com>

> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
> Sent: Thursday, December 12, 2024 6:08 AM
> To: Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Pillai, Aurabindo
> <Aurabindo.Pillai at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM at amd.com>; Li, Sun peng (Leo)
> <Sunpeng.Li at amd.com>; Chung, ChiaHsuan (Tom)
> <ChiaHsuan.Chung at amd.com>; Li, Roman <Roman.Li at amd.com>; Hung, Alex
> <Alex.Hung at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>; Hamza
> Mahfooz <hamza.mahfooz at amd.com>; Dan Carpenter
> <dan.carpenter at linaro.org>
> Subject: [PATCH v2] drm/amd/display: Fix NULL pointer dereference in
> dmub_tracebuffer_show
>
> It corrects the issue by checking if 'adev->dm.dmub_srv' is NULL before accessing
> its 'meta_info' member. This ensures that we do not dereference a NULL pointer.
>
> Fixes the below:
>       drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c
> :917 dmub_tracebuffer_show()
>       warn: address of 'adev->dm.dmub_srv->meta_info' is non-NULL
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_debugfs.c
>     901 static int dmub_tracebuffer_show(struct seq_file *m, void *data)
>     902 {
>     903         struct amdgpu_device *adev = m->private;
>     904         struct dmub_srv_fb_info *fb_info = adev->dm.dmub_fb_info;
>     905         struct dmub_fw_meta_info *fw_meta_info = &adev->dm.dmub_srv-
> >meta_info;
>                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Even if adev-
> >dm.dmub_srv is NULL, the address of ->meta_info can't be NULL
>
>     906         struct dmub_debugfs_trace_entry *entries;
>     907         uint8_t *tbuf_base;
>     908         uint32_t tbuf_size, max_entries, num_entries, first_entry, i;
>     909
>     910         if (!fb_info)
>     911                 return 0;
>     912
>     913         tbuf_base = (uint8_t *)fb_info-
> >fb[DMUB_WINDOW_5_TRACEBUFF].cpu_addr;
>     914         if (!tbuf_base)
>     915                 return 0;
>     916
> --> 917         tbuf_size = fw_meta_info ? fw_meta_info->trace_buffer_size :
>                             ^^^^^^^^^^^^ Always non-NULL
>
>     918                                    DMUB_TRACE_BUFFER_SIZE;
>     919         max_entries = (tbuf_size - sizeof(struct dmub_debugfs_trace_header)) /
>     920                       sizeof(struct dmub_debugfs_trace_entry);
>     921
>     922         num_entries =
>
> Fixes: c506f6e03b18 ("drm/amd/display: Make DMCUB tracebuffer debugfs
> chronological")
> Cc: Leo Li <sunpeng.li at amd.com>
> Cc: Tom Chung <chiahsuan.chung at amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Cc: Roman Li <roman.li at amd.com>
> Cc: Alex Hung <alex.hung at amd.com>
> Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Hamza Mahfooz <hamza.mahfooz at amd.com>
> Reported-by: Dan Carpenter <dan.carpenter at linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
> v2:
>  - initially initialize struct dmub_fw_meta_info *fw_meta_info to NULL (Dan
>    Carpenter)
>
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 11a7ac54f91c..2d31836ecb98 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -902,7 +902,7 @@ static int dmub_tracebuffer_show(struct seq_file *m, void
> *data)  {
>       struct amdgpu_device *adev = m->private;
>       struct dmub_srv_fb_info *fb_info = adev->dm.dmub_fb_info;
> -     struct dmub_fw_meta_info *fw_meta_info = &adev->dm.dmub_srv-
> >meta_info;
> +     struct dmub_fw_meta_info *fw_meta_info = NULL;
>       struct dmub_debugfs_trace_entry *entries;
>       uint8_t *tbuf_base;
>       uint32_t tbuf_size, max_entries, num_entries, first_entry, i; @@ -914,6
> +914,9 @@ static int dmub_tracebuffer_show(struct seq_file *m, void *data)
>       if (!tbuf_base)
>               return 0;
>
> +     if (adev->dm.dmub_srv)
> +             fw_meta_info = &adev->dm.dmub_srv->meta_info;
> +
>       tbuf_size = fw_meta_info ? fw_meta_info->trace_buffer_size :
>                                  DMUB_TRACE_BUFFER_SIZE;
>       max_entries = (tbuf_size - sizeof(struct dmub_debugfs_trace_header)) /
> --
> 2.34.1