On Fri, 2025-11-21 at 20:32 +0530, Srinivasan Shanmugam wrote:
> The function dereferences amdgpu_dm_connector->dc_link early to
> initialize verified_link_cap and dc, but later still checks
> amdgpu_dm_connector->dc_link for NULL in the analog path.
>
> This late NULL check is redundant, introduce a local dc_link pointer,
> use it consistently, and drop the superfluous NULL check while using
> dc_link->link_id.id instead.
>
> The function uses dc_link at the very beginning without checking if
> it
> is NULL. But later in the code, it suddenly checks if dc_link is
> NULL.
>
> This check is too late to be useful, because the code has already
> used
> dc_link earlier. So this NULL check does nothing.
>
> We simplify the code by storing amdgpu_dm_connector->dc_link in a
> local
> dc_link variable and using it throughout the function. Since dc_link
> is
> already dereferenced early, the later NULL check is unnecessary and
> is
> removed.
>
> Fixes the below:
> amdgpu_dm_connector_get_modes():
> variable dereferenced before check 'amdgpu_dm_connector->dc_link'
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
> 8845 &amdgpu_dm_connector->dc_link-
> >verified_link_cap;
> 8846 const struct dc *dc = amdgpu_dm_connector->dc_link-
> >dc;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Dereference
> ...
>
> 8856
> 8857 if (amdgpu_dm_connector->dc_sink &&
> 8858 amdgpu_dm_connector->dc_link &&
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Checked too late.
> Presumably this NULL check could be
> removed?
> ...
>
> Fixes: c8f52ca2cefb ("drm/amd/display: Cleanup uses of the analog
> flag")
> Reported by: Dan Carpenter <[email protected]>
> Cc: Timur Kristóf <[email protected]>
> Cc: Aurabindo Pillai <[email protected]>
> Cc: Roman Li <[email protected]>
> Cc: Harry Wentland <[email protected]>
> Cc: Tom Chung <[email protected]>
> Signed-off-by: Srinivasan Shanmugam <[email protected]>
Nice catch!
Reviewed-by: Timur Kristóf <[email protected]>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8a0555365719..6eb88a96cc33 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8818,11 +8818,11 @@ static int
> amdgpu_dm_connector_get_modes(struct drm_connector *connector)
> {
> struct amdgpu_dm_connector *amdgpu_dm_connector =
> to_amdgpu_dm_connector(connector);
> + struct dc_link *dc_link = amdgpu_dm_connector->dc_link;
> struct drm_encoder *encoder;
> const struct drm_edid *drm_edid = amdgpu_dm_connector-
> >drm_edid;
> - struct dc_link_settings *verified_link_cap =
> - &amdgpu_dm_connector->dc_link-
> >verified_link_cap;
> - const struct dc *dc = amdgpu_dm_connector->dc_link->dc;
> + struct dc_link_settings *verified_link_cap = &dc_link-
> >verified_link_cap;
> + const struct dc *dc = dc_link->dc;
>
> encoder = amdgpu_dm_connector_to_encoder(connector);
>
> @@ -8834,9 +8834,8 @@ static int amdgpu_dm_connector_get_modes(struct
> drm_connector *connector)
> drm_add_modes_noedid(connector,
> 1920, 1080);
>
> if (amdgpu_dm_connector->dc_sink &&
> - amdgpu_dm_connector->dc_link &&
> amdgpu_dm_connector->dc_sink->edid_caps.analog
> &&
> -
> dc_connector_supports_analog(amdgpu_dm_connector->dc_link-
> >link_id.id)) {
> + dc_connector_supports_analog(dc_link-
> >link_id.id)) {
> /* Analog monitor connected by DAC load
> detection.
> * Add common modes. It will be up to the
> user to select one that works.
> */
