[PATCH] drm/amd/display: Add 'replay' NULL check in 'edp_set_replay_allow_active()'

[PATCH] drm/amd/display: Add 'replay' NULL check in 'edp_set_replay_allow_active()'

[Srinivasan Shanmugam]

In the first if statement, we're checking if 'replay' is NULL. But in
the second if statement, we're not checking if 'replay' is NULL again
before calling replay->funcs->replay_set_power_opt().

if (replay == NULL && force_static)
    return false;

...

if (link->replay_settings.replay_feature_enabled &&
    replay->funcs->replay_set_power_opt) {
	replay->funcs->replay_set_power_opt(replay, *power_opts, panel_inst);
	link->replay_settings.replay_power_opt_active = *power_opts;
}

If 'replay' is NULL, this will cause a null pointer dereference.

Fixes the below found by smatch:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:895 edp_set_replay_allow_active() error: we previously assumed 'replay' could be null (see line 887)

Fixes: c7ddc0a800bc ("drm/amd/display: Add Functions to enable Freesync Panel Replay")
Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Cc: Roman Li <roman.li@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Cc: Tom Chung <chiahsuan.chung@amd.com>
Suggested-by: Tom Chung <chiahsuan.chung@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 .../drm/amd/display/dc/link/protocols/link_edp_panel_control.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index 443215b96308..acfbbc638cc6 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -892,7 +892,8 @@ bool edp_set_replay_allow_active(struct dc_link *link, const bool *allow_active,
 
 	/* Set power optimization flag */
 	if (power_opts && link->replay_settings.replay_power_opt_active != *power_opts) {
-		if (link->replay_settings.replay_feature_enabled && replay->funcs->replay_set_power_opt) {
+		if (replay != NULL && link->replay_settings.replay_feature_enabled &&
+		    replay->funcs->replay_set_power_opt) {
 			replay->funcs->replay_set_power_opt(replay, *power_opts, panel_inst);
 			link->replay_settings.replay_power_opt_active = *power_opts;
 		}
-- 
2.34.1

Re: [PATCH] drm/amd/display: Add 'replay' NULL check in 'edp_set_replay_allow_active()'

[Chung, ChiaHsuan (Tom)]

[-- Attachment #1: Type: text/plain, Size: 2433 bytes --]

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

On 2/15/2024 9:31 PM, Srinivasan Shanmugam wrote:
> In the first if statement, we're checking if 'replay' is NULL. But in
> the second if statement, we're not checking if 'replay' is NULL again
> before calling replay->funcs->replay_set_power_opt().
>
> if (replay == NULL && force_static)
>      return false;
>
> ...
>
> if (link->replay_settings.replay_feature_enabled &&
>      replay->funcs->replay_set_power_opt) {
> 	replay->funcs->replay_set_power_opt(replay, *power_opts, panel_inst);
> 	link->replay_settings.replay_power_opt_active = *power_opts;
> }
>
> If 'replay' is NULL, this will cause a null pointer dereference.
>
> Fixes the below found by smatch:
> drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_edp_panel_control.c:895 edp_set_replay_allow_active() error: we previously assumed 'replay' could be null (see line 887)
>
> Fixes: c7ddc0a800bc ("drm/amd/display: Add Functions to enable Freesync Panel Replay")
> Cc: Bhawanpreet Lakha<Bhawanpreet.Lakha@amd.com>
> Cc: Roman Li<roman.li@amd.com>
> Cc: Rodrigo Siqueira<Rodrigo.Siqueira@amd.com>
> Cc: Aurabindo Pillai<aurabindo.pillai@amd.com>
> Cc: Tom Chung<chiahsuan.chung@amd.com>
> Suggested-by: Tom Chung<chiahsuan.chung@amd.com>
> Signed-off-by: Srinivasan Shanmugam<srinivasan.shanmugam@amd.com>
> ---
>   .../drm/amd/display/dc/link/protocols/link_edp_panel_control.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> index 443215b96308..acfbbc638cc6 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
> @@ -892,7 +892,8 @@ bool edp_set_replay_allow_active(struct dc_link *link, const bool *allow_active,
>   
>   	/* Set power optimization flag */
>   	if (power_opts && link->replay_settings.replay_power_opt_active != *power_opts) {
> -		if (link->replay_settings.replay_feature_enabled && replay->funcs->replay_set_power_opt) {
> +		if (replay != NULL && link->replay_settings.replay_feature_enabled &&
> +		    replay->funcs->replay_set_power_opt) {
>   			replay->funcs->replay_set_power_opt(replay, *power_opts, panel_inst);
>   			link->replay_settings.replay_power_opt_active = *power_opts;
>   		}

[-- Attachment #2: Type: text/html, Size: 3830 bytes --]