Re: [PATCH] drm/amd/display: Avoid NULL dereference in dc_dmub_srv error paths

[Alex Hung <alex.hung@amd.com>]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 3/23/26 01:32, Srinivasan Shanmugam wrote:
> In dc_dmub_srv_log_diagnostic_data() and
> dc_dmub_srv_enable_dpia_trace().
> 
> Both functions check:
> 
>    if (!dc_dmub_srv || !dc_dmub_srv->dmub)
> 
> and then call DC_LOG_ERROR() inside that block.
> 
> DC_LOG_ERROR() uses dc_dmub_srv->ctx internally. So if
> dc_dmub_srv is NULL, the logging itself can dereference a
> NULL pointer and cause a crash.
> 
> Fix this by splitting the checks.
> 
> First check if dc_dmub_srv is NULL and return immediately.
> Then check dc_dmub_srv->dmub and log the error only when
> dc_dmub_srv is valid.
> 
> Fixes the below:
> ../display/dc/dc_dmub_srv.c:962 dc_dmub_srv_log_diagnostic_data() error: we previously assumed 'dc_dmub_srv' could be null (see line 961)
> ../display/dc/dc_dmub_srv.c:1167 dc_dmub_srv_enable_dpia_trace() error: we previously assumed 'dc_dmub_srv' could be null (see line 1166)
> 
> Fixes: 2631ac1ac328 ("drm/amd/display: add DMUB registers to crash dump diagnostic data.")
> Fixes: 71ba6b577a35 ("drm/amd/display: Add interface to enable DPIA trace")
> Cc: Roman Li <roman.li@amd.com>
> Cc: Alex Hung <alex.hung@amd.com>
> Cc: Tom Chung <chiahsuan.chung@amd.com>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>   drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> index d6a08064456b..cab8fb4c56d0 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> @@ -958,7 +958,10 @@ void dc_dmub_srv_log_diagnostic_data(struct dc_dmub_srv *dc_dmub_srv)
>   {
>   	uint32_t i;
>   
> -	if (!dc_dmub_srv || !dc_dmub_srv->dmub) {
> +	if (!dc_dmub_srv)
> +		return;
> +
> +	if (!dc_dmub_srv->dmub) {
>   		DC_LOG_ERROR("%s: invalid parameters.", __func__);
>   		return;
>   	}
> @@ -1163,7 +1166,10 @@ void dc_dmub_srv_enable_dpia_trace(const struct dc *dc)
>   {
>   	struct dc_dmub_srv *dc_dmub_srv = dc->ctx->dmub_srv;
>   
> -	if (!dc_dmub_srv || !dc_dmub_srv->dmub) {
> +	if (!dc_dmub_srv)
> +		return;
> +
> +	if (!dc_dmub_srv->dmub) {
>   		DC_LOG_ERROR("%s: invalid parameters.", __func__);
>   		return;
>   	}