amdgpu_dm_connector_ddc_get_modes() reinitializes a connector's probed
modes list without cleaning it up. First time it is called during the
driver's initialization phase, then via drm_mode_getconnector() ioctl.
The leaks observed with Kmemleak are as following:
unreferenced object 0xffff88812f91b200 (size 128):
comm "(udev-worker)", pid 388, jiffies 4294695475
hex dump (first 32 bytes):
ac dd 07 00 80 02 70 0b 90 0b e0 0b 00 00 e0 01 ......p.........
0b 07 10 07 5c 07 00 00 0a 00 00 00 00 00 00 00 ....\...........
backtrace (crc 89db554f):
__kmalloc_cache_noprof+0x3a3/0x490
drm_mode_duplicate+0x8e/0x2b0
amdgpu_dm_create_common_mode+0x40/0x150 [amdgpu]
amdgpu_dm_connector_add_common_modes+0x336/0x488 [amdgpu]
amdgpu_dm_connector_get_modes+0x428/0x8a0 [amdgpu]
amdgpu_dm_initialize_drm_device+0x1389/0x17b4 [amdgpu]
amdgpu_dm_init.cold+0x157b/0x1a1e [amdgpu]
dm_hw_init+0x3f/0x110 [amdgpu]
amdgpu_device_ip_init+0xcf4/0x1180 [amdgpu]
amdgpu_device_init.cold+0xb84/0x1863 [amdgpu]
amdgpu_driver_load_kms+0x15/0x90 [amdgpu]
amdgpu_pci_probe+0x391/0xce0 [amdgpu]
local_pci_probe+0xd9/0x190
pci_call_probe+0x183/0x540
pci_device_probe+0x171/0x2c0
really_probe+0x1e1/0x890
Found by Linux Verification Center (linuxtesting.org).
Fixes: acc96ae0d127 ("drm/amd/display: set panel orientation before drm_dev_register")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
1 file changed, 3 insertions(+)
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 cd0e2976e268..7ec1f9afc081 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8227,9 +8227,12 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
{
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
+ struct drm_display_mode *mode, *t;
if (drm_edid) {
/* empty probed_modes */
+ list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
+ drm_mode_remove(connector, mode);
INIT_LIST_HEAD(&connector->probed_modes);
amdgpu_dm_connector->num_modes =
drm_edid_connector_add_modes(connector);
--
2.50.1
On 19/08/2025 15:46, Fedor Pchelkin wrote:
> amdgpu_dm_connector_ddc_get_modes() reinitializes a connector's probed
> modes list without cleaning it up. First time it is called during the
> driver's initialization phase, then via drm_mode_getconnector() ioctl.
> The leaks observed with Kmemleak are as following:
>
> unreferenced object 0xffff88812f91b200 (size 128):
> comm "(udev-worker)", pid 388, jiffies 4294695475
> hex dump (first 32 bytes):
> ac dd 07 00 80 02 70 0b 90 0b e0 0b 00 00 e0 01 ......p.........
> 0b 07 10 07 5c 07 00 00 0a 00 00 00 00 00 00 00 ....\...........
> backtrace (crc 89db554f):
> __kmalloc_cache_noprof+0x3a3/0x490
> drm_mode_duplicate+0x8e/0x2b0
> amdgpu_dm_create_common_mode+0x40/0x150 [amdgpu]
> amdgpu_dm_connector_add_common_modes+0x336/0x488 [amdgpu]
> amdgpu_dm_connector_get_modes+0x428/0x8a0 [amdgpu]
> amdgpu_dm_initialize_drm_device+0x1389/0x17b4 [amdgpu]
> amdgpu_dm_init.cold+0x157b/0x1a1e [amdgpu]
> dm_hw_init+0x3f/0x110 [amdgpu]
> amdgpu_device_ip_init+0xcf4/0x1180 [amdgpu]
> amdgpu_device_init.cold+0xb84/0x1863 [amdgpu]
> amdgpu_driver_load_kms+0x15/0x90 [amdgpu]
> amdgpu_pci_probe+0x391/0xce0 [amdgpu]
> local_pci_probe+0xd9/0x190
> pci_call_probe+0x183/0x540
> pci_device_probe+0x171/0x2c0
> really_probe+0x1e1/0x890
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Fixes: acc96ae0d127 ("drm/amd/display: set panel orientation before drm_dev_register")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> 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 cd0e2976e268..7ec1f9afc081 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8227,9 +8227,12 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
> {
> struct amdgpu_dm_connector *amdgpu_dm_connector =
> to_amdgpu_dm_connector(connector);
> + struct drm_display_mode *mode, *t;
>
> if (drm_edid) {
> /* empty probed_modes */
> + list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
> + drm_mode_remove(connector, mode);
> INIT_LIST_HEAD(&connector->probed_modes);
> amdgpu_dm_connector->num_modes =
> drm_edid_connector_add_modes(connector);
What if you update the connector with the drm_edid data and skip the
INIT_LIST_HEAD instead?
Something like:
if (drm_edid) {
drm_edid_connector_update(connector, drm_edid);
amdgpu_drm_connector->num_modes =
drm_edid_connector_add_modes(connector);
[...]
}
Isn't it enough?
Melissa
Hi,
On Wed, 20. Aug 13:00, Melissa Wen wrote:
> On 19/08/2025 15:46, Fedor Pchelkin wrote:
> > amdgpu_dm_connector_ddc_get_modes() reinitializes a connector's probed
> > modes list without cleaning it up. First time it is called during the
> > driver's initialization phase, then via drm_mode_getconnector() ioctl.
> > The leaks observed with Kmemleak are as following:
> >
> > unreferenced object 0xffff88812f91b200 (size 128):
> > comm "(udev-worker)", pid 388, jiffies 4294695475
> > hex dump (first 32 bytes):
> > ac dd 07 00 80 02 70 0b 90 0b e0 0b 00 00 e0 01 ......p.........
> > 0b 07 10 07 5c 07 00 00 0a 00 00 00 00 00 00 00 ....\...........
> > backtrace (crc 89db554f):
> > __kmalloc_cache_noprof+0x3a3/0x490
> > drm_mode_duplicate+0x8e/0x2b0
> > amdgpu_dm_create_common_mode+0x40/0x150 [amdgpu]
> > amdgpu_dm_connector_add_common_modes+0x336/0x488 [amdgpu]
> > amdgpu_dm_connector_get_modes+0x428/0x8a0 [amdgpu]
> > amdgpu_dm_initialize_drm_device+0x1389/0x17b4 [amdgpu]
> > amdgpu_dm_init.cold+0x157b/0x1a1e [amdgpu]
> > dm_hw_init+0x3f/0x110 [amdgpu]
> > amdgpu_device_ip_init+0xcf4/0x1180 [amdgpu]
> > amdgpu_device_init.cold+0xb84/0x1863 [amdgpu]
> > amdgpu_driver_load_kms+0x15/0x90 [amdgpu]
> > amdgpu_pci_probe+0x391/0xce0 [amdgpu]
> > local_pci_probe+0xd9/0x190
> > pci_call_probe+0x183/0x540
> > pci_device_probe+0x171/0x2c0
> > really_probe+0x1e1/0x890
> >
> > Found by Linux Verification Center (linuxtesting.org).
> >
> > Fixes: acc96ae0d127 ("drm/amd/display: set panel orientation before drm_dev_register")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> > ---
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > 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 cd0e2976e268..7ec1f9afc081 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -8227,9 +8227,12 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
> > {
> > struct amdgpu_dm_connector *amdgpu_dm_connector =
> > to_amdgpu_dm_connector(connector);
> > + struct drm_display_mode *mode, *t;
> > if (drm_edid) {
> > /* empty probed_modes */
> > + list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
> > + drm_mode_remove(connector, mode);
> > INIT_LIST_HEAD(&connector->probed_modes);
> > amdgpu_dm_connector->num_modes =
> > drm_edid_connector_add_modes(connector);
>
> What if you update the connector with the drm_edid data and skip the
> INIT_LIST_HEAD instead?
Yep, getting rid of INIT_LIST_HEAD eliminates the leak, too.
drm_edid_connector_add_modes() comments do also strongly recommend calling
drm_edid_connector_update() before the function.
One thing remaining strange is that there'd be several different objects
in the probed_modes list describing the same things I guess.
>
> Something like:
>
> if (drm_edid) {
At this point we already have the modes in the list added with the
previous call to amdgpu_dm_connector_get_modes() from
amdgpu_set_panel_orientation() - during the driver initialization phase.
> drm_edid_connector_update(connector, drm_edid);
> amdgpu_drm_connector->num_modes =
> drm_edid_connector_add_modes(connector);
Here we add them again (as new objects) to the list. By the way it leads
to amdgpu_drm_connector->num_modes be less than the actual number of
elements present in probed_modes list.
As far as I understand, *_get_modes() are supposed to be called only via
drm_mode_get_connector ioctl, and not all things go as expected if they're
firstly called in another path, as e.g. in amdgpu case through
amdgpu_set_panel_orientation().
But it seems commit acc96ae0d127 ("drm/amd/display: set panel orientation
before drm_dev_register") added that call deliberately.
I think we may update the connector with the drm_edid data and skip the
INIT_LIST_HEAD part as you've suggested, but also need to flush the list -
it might contain something left from the first amdgpu_dm_connector_get_modes()
call.
If no objections, I'll send it out as v3 soon.
> [...]
> }
>
> Isn't it enough?
© 2016 - 2026 Red Hat, Inc.