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 ---
I can't reproduce the issue before the commit in Fixes which placed that extra amdgpu_dm_connector_get_modes() call. Though the reinitializing part
/* empty probed_modes */ INIT_LIST_HEAD(&connector->probed_modes);
was added years before and it looks OK since drm_connector_list_update() should shake the list in between drm_mode_getconnector() ioctl calls.
For what the patch does there exists a drm_mode_remove() helper but it's static at drivers/gpu/drm/drm_connector.c and requires to be exported first. This probably looks like a subject for an independent for-next patch, if needed.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++++ 1 file changed, 5 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..4b84f944f066 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,14 @@ 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) { + list_del(&mode->head); + drm_mode_destroy(connector->dev, mode); + } INIT_LIST_HEAD(&connector->probed_modes); amdgpu_dm_connector->num_modes = drm_edid_connector_add_modes(connector);
On 8/17/25 4:43 AM, 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
I can't reproduce the issue before the commit in Fixes which placed that extra amdgpu_dm_connector_get_modes() call. Though the reinitializing part
/* empty probed_modes */ INIT_LIST_HEAD(&connector->probed_modes);
was added years before and it looks OK since drm_connector_list_update() should shake the list in between drm_mode_getconnector() ioctl calls.
For what the patch does there exists a drm_mode_remove() helper but it's static at drivers/gpu/drm/drm_connector.c and requires to be exported first. This probably looks like a subject for an independent for-next patch, if needed.
So you're saying this change will take two iterations of patches to ping pong the code?
Why not just send this as a two patch series?
1) Export the symbol drm_mode_remove() 2) This patch, but use the symbol.
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++++ 1 file changed, 5 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..4b84f944f066 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,14 @@ 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) {list_del(&mode->head);drm_mode_destroy(connector->dev, mode);}
On Mon, 18. Aug 21:17, Limonciello, Mario wrote:
On 8/17/25 4:43 AM, Fedor Pchelkin wrote:
For what the patch does there exists a drm_mode_remove() helper but it's static at drivers/gpu/drm/drm_connector.c and requires to be exported first. This probably looks like a subject for an independent for-next patch, if needed.
So you're saying this change will take two iterations of patches to ping pong the code?
Why not just send this as a two patch series?
- Export the symbol drm_mode_remove()
- This patch, but use the symbol.
Initially I wasn't sure if the exporting patch was worth moving the code around and in the end decided to make the current patch with a minimum of prerequisites.
But giving this a second glance, I see exporting the symbol would be technically better. I'll send out v2.
Thanks!
linux-stable-mirror@lists.linaro.org
-
Fedor Pchelkin -
Limonciello, Mario