[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <79a7f64c-4afa-441d-b1be-bab489174c7e@amd.com>
Date: Mon, 18 Aug 2025 21:17:01 +0000
From: "Limonciello, Mario" <Mario.Limonciello@....com>
To: Fedor Pchelkin <pchelkin@...ras.ru>, "Deucher, Alexander"
<Alexander.Deucher@....com>, Melissa Wen <mwen@...lia.com>
CC: "Wentland, Harry" <Harry.Wentland@....com>, Rodrigo Siqueira
<siqueira@...lia.com>, "Koenig, Christian" <Christian.Koenig@....com>, David
Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Hans de Goede
<hansg@...nel.org>, "amd-gfx@...ts.freedesktop.org"
<amd-gfx@...ts.freedesktop.org>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "lvc-project@...uxtesting.org"
<lvc-project@...uxtesting.org>, "stable@...r.kernel.org"
<stable@...r.kernel.org>
Subject: Re: [PATCH] drm/amd/display: fix leak of probed modes
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@...r.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.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);
> + }
> INIT_LIST_HEAD(&connector->probed_modes);
> amdgpu_dm_connector->num_modes =
> drm_edid_connector_add_modes(connector);
Powered by blists - more mailing lists