* [PATCH] drm/amd/display: Fix dereference-before-check for dc_link
@ 2025-11-21 15:02 Srinivasan Shanmugam
2025-11-21 16:43 ` Alex Hung
2025-11-23 21:55 ` Timur Kristóf
0 siblings, 2 replies; 3+ messages in thread
From: Srinivasan Shanmugam @ 2025-11-21 15:02 UTC (permalink / raw)
To: Alex Hung, Aurabindo Pillai
Cc: amd-gfx, Srinivasan Shanmugam, Timur Kristóf, Roman Li,
Harry Wentland, Tom Chung
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 <dan.carpenter@linaro.org>
Cc: Timur Kristóf <timur.kristof@gmail.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Cc: Roman Li <roman.li@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Tom Chung <chiahsuan.chung@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
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.
*/
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amd/display: Fix dereference-before-check for dc_link
2025-11-21 15:02 [PATCH] drm/amd/display: Fix dereference-before-check for dc_link Srinivasan Shanmugam
@ 2025-11-21 16:43 ` Alex Hung
2025-11-23 21:55 ` Timur Kristóf
1 sibling, 0 replies; 3+ messages in thread
From: Alex Hung @ 2025-11-21 16:43 UTC (permalink / raw)
To: Srinivasan Shanmugam, Aurabindo Pillai
Cc: amd-gfx, Timur Kristóf, Roman Li, Harry Wentland, Tom Chung
Hi Srinivasan,
The patch looks good and I will send it to promotion test next week.
On 11/21/25 08:02, 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 <dan.carpenter@linaro.org>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Cc: Roman Li <roman.li@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Tom Chung <chiahsuan.chung@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
> 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.
> */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amd/display: Fix dereference-before-check for dc_link
2025-11-21 15:02 [PATCH] drm/amd/display: Fix dereference-before-check for dc_link Srinivasan Shanmugam
2025-11-21 16:43 ` Alex Hung
@ 2025-11-23 21:55 ` Timur Kristóf
1 sibling, 0 replies; 3+ messages in thread
From: Timur Kristóf @ 2025-11-23 21:55 UTC (permalink / raw)
To: Srinivasan Shanmugam, Alex Hung, Aurabindo Pillai
Cc: amd-gfx, Roman Li, Harry Wentland, Tom Chung
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 <dan.carpenter@linaro.org>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Cc: Roman Li <roman.li@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Tom Chung <chiahsuan.chung@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Nice catch!
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
> 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.
> */
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-23 21:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-21 15:02 [PATCH] drm/amd/display: Fix dereference-before-check for dc_link Srinivasan Shanmugam
2025-11-21 16:43 ` Alex Hung
2025-11-23 21:55 ` Timur Kristóf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox