[PATCH v2] drm/amd/display: Fix potential null pointer dereference in dc_dmub_srv

[Chung, ChiaHsuan (Tom)]

Reviewed-by: Tom Chung <chiahsuan.chung at amd.com>

On 2/20/2024 5:06 PM, Srinivasan Shanmugam wrote:
> Fixes potential null pointer dereference warnings in the
> dc_dmub_srv_cmd_list_queue_execute() and dc_dmub_srv_is_hw_pwr_up()
> functions.
>
> In both functions, the 'dc_dmub_srv' variable was being dereferenced
> before it was checked for null. This could lead to a null pointer
> dereference if 'dc_dmub_srv' is null. The fix is to check if
> 'dc_dmub_srv' is null before dereferencing it.
>
> Thus moving the null checks for 'dc_dmub_srv' to the beginning of the
> functions to ensure that 'dc_dmub_srv' is not null when it is
> dereferenced.
>
> Found by smatch & thus fixing the below:
> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:133 dc_dmub_srv_cmd_list_queue_execute() warn: variable dereferenced before check 'dc_dmub_srv' (see line 128)
> drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.c:1167 dc_dmub_srv_is_hw_pwr_up() warn: variable dereferenced before check 'dc_dmub_srv' (see line 1164)
>
> Fixes: 01fbdc34c687 ("drm/amd/display: decouple dmcub execution to reduce lock granularity")
> Fixes: 65138eb72e1f ("drm/amd/display: Add DCN35 DMUB")
> Cc: JinZe.Xu <jinze.xu at amd.com>
> Cc: Hersen Wu <hersenxs.wu at amd.com>
> Cc: Josip Pavic <josip.pavic at amd.com>
> Cc: Roman Li <roman.li at amd.com>
> Cc: Qingqing Zhuo <Qingqing.Zhuo at amd.com>
> Cc: Harry Wentland <Harry.Wentland at amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
> Cc: Tom Chung <chiahsuan.chung at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
> v2:
>   - For dc_dmub_srv_is_hw_pwr_up() move 'dc_ctx = dc_dmub_srv->ctx;'
>     below 'if (dc_dmub_srv->ctx->dc->debug.dmcub_emulation)' (Tom)
>
>   drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 7 +++++--
>   1 file changed, 5 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 0bc32537e2eb..a115e1170ef5 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
> @@ -128,7 +128,7 @@ bool dc_dmub_srv_cmd_list_queue_execute(struct dc_dmub_srv *dc_dmub_srv,
>   		unsigned int count,
>   		union dmub_rb_cmd *cmd_list)
>   {
> -	struct dc_context *dc_ctx = dc_dmub_srv->ctx;
> +	struct dc_context *dc_ctx;
>   	struct dmub_srv *dmub;
>   	enum dmub_status status;
>   	int i;
> @@ -136,6 +136,7 @@ bool dc_dmub_srv_cmd_list_queue_execute(struct dc_dmub_srv *dc_dmub_srv,
>   	if (!dc_dmub_srv || !dc_dmub_srv->dmub)
>   		return false;
>   
> +	dc_ctx = dc_dmub_srv->ctx;
>   	dmub = dc_dmub_srv->dmub;
>   
>   	for (i = 0 ; i < count; i++) {
> @@ -1169,7 +1170,7 @@ void dc_dmub_srv_subvp_save_surf_addr(const struct dc_dmub_srv *dc_dmub_srv, con
>   
>   bool dc_dmub_srv_is_hw_pwr_up(struct dc_dmub_srv *dc_dmub_srv, bool wait)
>   {
> -	struct dc_context *dc_ctx = dc_dmub_srv->ctx;
> +	struct dc_context *dc_ctx;
>   	enum dmub_status status;
>   
>   	if (!dc_dmub_srv || !dc_dmub_srv->dmub)
> @@ -1177,6 +1178,8 @@ bool dc_dmub_srv_is_hw_pwr_up(struct dc_dmub_srv *dc_dmub_srv, bool wait)
>   
>   	if (dc_dmub_srv->ctx->dc->debug.dmcub_emulation)
>   		return true;
> +
> +	dc_ctx = dc_dmub_srv->ctx;
>   
>   	if (wait) {
>   		if (dc_dmub_srv->ctx->dc->debug.disable_timeout) {