From: "Joshua Peisach" <jpeisach@ubuntu.com>
To: "SHANMUGAM, SRINIVASAN" <SRINIVASAN.SHANMUGAM@amd.com>,
"Dan Carpenter" <error27@gmail.com>,
"Deucher, Alexander" <Alexander.Deucher@amd.com>
Cc: "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
"Wentland, Harry" <Harry.Wentland@amd.com>,
"Li, Roman" <Roman.Li@amd.com>, "Hung, Alex" <Alex.Hung@amd.com>,
"Chung, ChiaHsuan (Tom)" <ChiaHsuan.Chung@amd.com>,
"Pillai, Aurabindo" <Aurabindo.Pillai@amd.com>
Subject: Re: [bug report] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid
Date: Sat, 11 Apr 2026 07:45:19 -0400 [thread overview]
Message-ID: <DHQATFD3MPLS.XULA16D1IROS@ubuntu.com> (raw)
In-Reply-To: <IA0PR12MB8208DAB29B0617E8F6D082C490262@IA0PR12MB8208.namprd12.prod.outlook.com>
On Sat Apr 11, 2026 at 1:30 AM EDT, SRINIVASAN SHANMUGAM wrote:
> I went through the code path, and the warning looks valid:
>
> In amdgpu_connector_dvi_detect(), we do:
> drm_edid_free(amdgpu_connector->edid);
> After that, we call:
> amdgpu_connector_get_edid(connector);
> But inside amdgpu_connector_get_edid():
> It immediately returns if amdgpu_connector->edid is non-NULL
> Since we did not set amdgpu_connector->edid = NULL after freeing:
> The pointer is still non-NULL (but already freed)
> So amdgpu_connector_get_edid() becomes a no-op
> No new EDID is read
> Then later we do:
> drm_edid_is_digital(amdgpu_connector->edid);
> At this point:
> amdgpu_connector->edid still points to freed memory
> So this becomes a real use-after-free
>
> So the issue is not just the removal of amdgpu_connector_free_edid(),
> but that we lost the behavior of clearing the cached EDID pointer after free.
>
> Because of this, the EDID cache logic breaks.
>
> About reverting:
>
> Reverting the commit would fix it indirectly
> But I think a minimal fix is better:
> Set amdgpu_connector->edid = NULL after drm_edid_free()
>
> This keeps the current design intact and fixes the bug cleanly.
>
I like it - less is more :) and it also makes it clear that the
pointer is set to NULL, instead of being hidden behind a function.
-Josh
prev parent reply other threads:[~2026-04-11 11:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-10 7:32 [bug report] drm/amdgpu/amdgpu_connectors: remove amdgpu_connector_free_edid Dan Carpenter
2026-04-10 12:00 ` Joshua Peisach
2026-04-11 5:30 ` SHANMUGAM, SRINIVASAN
2026-04-11 11:45 ` Joshua Peisach [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DHQATFD3MPLS.XULA16D1IROS@ubuntu.com \
--to=jpeisach@ubuntu.com \
--cc=Alex.Hung@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=Aurabindo.Pillai@amd.com \
--cc=ChiaHsuan.Chung@amd.com \
--cc=Harry.Wentland@amd.com \
--cc=Roman.Li@amd.com \
--cc=SRINIVASAN.SHANMUGAM@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=error27@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.