[Public]
Reviewed-by: Roman Li <roman.li@xxxxxxx>
> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>
> Sent: Thursday, December 12, 2024 6:08 AM
> To: Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx>; Pillai, Aurabindo
> <Aurabindo.Pillai@xxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM@xxxxxxx>; Li, Sun peng (Leo)
> <Sunpeng.Li@xxxxxxx>; Chung, ChiaHsuan (Tom)
> <ChiaHsuan.Chung@xxxxxxx>; Li, Roman <Roman.Li@xxxxxxx>; Hung, Alex
> <Alex.Hung@xxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>; Hamza
> Mahfooz <hamza.mahfooz@xxxxxxx>; Dan Carpenter
> <dan.carpenter@xxxxxxxxxx>
> 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@xxxxxxx>
> Cc: Tom Chung <chiahsuan.chung@xxxxxxx>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx>
> Cc: Roman Li <roman.li@xxxxxxx>
> Cc: Alex Hung <alex.hung@xxxxxxx>
> Cc: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Cc: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> ---
> 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
