[PATCH v2 0/2] a fix for connector modes leak in amdgpu driver

[PATCH v2 0/2] a fix for connector modes leak in amdgpu driver

[Fedor Pchelkin]

The first patch is a prerequisite exporting a convenient helper at
include/drm/modes.h used in the second one fixing a leak in amdgpu
driver.

v1: https://lore.kernel.org/all/20250817094346.15740-1-pchelkin@ispras.ru/

Fedor Pchelkin (2):
  drm/modes: export drm_mode_remove() helper
  drm/amd/display: fix leak of probed modes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  3 +++
 drivers/gpu/drm/drm_connector.c                   |  8 +-------
 drivers/gpu/drm/drm_modes.c                       | 15 +++++++++++++++
 include/drm/drm_modes.h                           |  1 +
 4 files changed, 20 insertions(+), 7 deletions(-)

-- 
2.50.1

[PATCH v2 1/2] drm/modes: export drm_mode_remove() helper

[Fedor Pchelkin]

This functionality may be helpful in drivers so export it for reusability.

Found by Linux Verification Center (linuxtesting.org).

Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
 drivers/gpu/drm/drm_connector.c |  8 +-------
 drivers/gpu/drm/drm_modes.c     | 15 +++++++++++++++
 include/drm/drm_modes.h         |  1 +
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 272d6254ea47..0a3933b6ceec 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -27,6 +27,7 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_file.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_modes.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_consumer.h>
@@ -696,13 +697,6 @@ bool drm_connector_has_possible_encoder(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_has_possible_encoder);
 
-static void drm_mode_remove(struct drm_connector *connector,
-			    struct drm_display_mode *mode)
-{
-	list_del(&mode->head);
-	drm_mode_destroy(connector->dev, mode);
-}
-
 /**
  * drm_connector_cec_phys_addr_invalidate - invalidate CEC physical address
  * @connector: connector undergoing CEC operation
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index e72f855fc495..6e021328f9c2 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -117,6 +117,21 @@ void drm_mode_probed_add(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_probed_add);
 
+/**
+ * drm_mode_remove - remove a mode from the associated list and destroy it
+ * @connector: connector of the mode
+ * @mode: mode data
+ *
+ * Remove @mode from the associated list, then free @mode object itself.
+ */
+void drm_mode_remove(struct drm_connector *connector,
+		     struct drm_display_mode *mode)
+{
+	list_del(&mode->head);
+	drm_mode_destroy(connector->dev, mode);
+}
+EXPORT_SYMBOL(drm_mode_remove);
+
 enum drm_mode_analog {
 	DRM_MODE_ANALOG_NTSC, /* 525 lines, 60Hz */
 	DRM_MODE_ANALOG_PAL, /* 625 lines, 50Hz */
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index b9bb92e4b029..d7b66321f60d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -460,6 +460,7 @@ int drm_mode_convert_umode(struct drm_device *dev,
 			   struct drm_display_mode *out,
 			   const struct drm_mode_modeinfo *in);
 void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode);
+void drm_mode_remove(struct drm_connector *connector, struct drm_display_mode *mode);
 void drm_mode_debug_printmodeline(const struct drm_display_mode *mode);
 bool drm_mode_is_420_only(const struct drm_display_info *display,
 			  const struct drm_display_mode *mode);
-- 
2.50.1

[PATCH v2 2/2] drm/amd/display: fix leak of probed modes

[Fedor Pchelkin]

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

Re: [PATCH v2 2/2] drm/amd/display: fix leak of probed modes

[Melissa Wen]

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

Re: [PATCH v2 2/2] drm/amd/display: fix leak of probed modes

[Fedor Pchelkin]

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?