diff mbox series

[2/2] drm/client: Fix memory leak in drm_client_modeset_probe

Message ID 20230711092203.68157-3-jfalempe@redhat.com (mailing list archive)
State New, archived
Headers show
Series Two memory leak fixes in drm_client_modeset.c | expand

Commit Message

Jocelyn Falempe July 11, 2023, 9:20 a.m. UTC
When a new mode is set to modeset->mode, the previous mode should be freed.
This fixes the following kmemleak report:

drm_mode_duplicate+0x45/0x220 [drm]
drm_client_modeset_probe+0x944/0xf50 [drm]
__drm_fb_helper_initial_config_and_unlock+0xb4/0x2c0 [drm_kms_helper]
drm_fbdev_client_hotplug+0x2bc/0x4d0 [drm_kms_helper]
drm_client_register+0x169/0x240 [drm]
ast_pci_probe+0x142/0x190 [ast]
local_pci_probe+0xdc/0x180
work_for_cpu_fn+0x4e/0xa0
process_one_work+0x8b7/0x1540
worker_thread+0x70a/0xed0
kthread+0x29f/0x340
ret_from_fork+0x1f/0x30

cc: <stable@vger.kernel.org>
Reported-by: Zhang Yi <yizhan@redhat.com>
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/drm_client_modeset.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Javier Martinez Canillas July 11, 2023, 9:29 a.m. UTC | #1
Jocelyn Falempe <jfalempe@redhat.com> writes:

> When a new mode is set to modeset->mode, the previous mode should be freed.
> This fixes the following kmemleak report:
>
> drm_mode_duplicate+0x45/0x220 [drm]
> drm_client_modeset_probe+0x944/0xf50 [drm]
> __drm_fb_helper_initial_config_and_unlock+0xb4/0x2c0 [drm_kms_helper]
> drm_fbdev_client_hotplug+0x2bc/0x4d0 [drm_kms_helper]
> drm_client_register+0x169/0x240 [drm]
> ast_pci_probe+0x142/0x190 [ast]
> local_pci_probe+0xdc/0x180
> work_for_cpu_fn+0x4e/0xa0
> process_one_work+0x8b7/0x1540
> worker_thread+0x70a/0xed0
> kthread+0x29f/0x340
> ret_from_fork+0x1f/0x30
>
> cc: <stable@vger.kernel.org>
> Reported-by: Zhang Yi <yizhan@redhat.com>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann July 11, 2023, 9:43 a.m. UTC | #2
Am 11.07.23 um 11:20 schrieb Jocelyn Falempe:
> When a new mode is set to modeset->mode, the previous mode should be freed.
> This fixes the following kmemleak report:
> 
> drm_mode_duplicate+0x45/0x220 [drm]
> drm_client_modeset_probe+0x944/0xf50 [drm]
> __drm_fb_helper_initial_config_and_unlock+0xb4/0x2c0 [drm_kms_helper]
> drm_fbdev_client_hotplug+0x2bc/0x4d0 [drm_kms_helper]
> drm_client_register+0x169/0x240 [drm]
> ast_pci_probe+0x142/0x190 [ast]
> local_pci_probe+0xdc/0x180
> work_for_cpu_fn+0x4e/0xa0
> process_one_work+0x8b7/0x1540
> worker_thread+0x70a/0xed0
> kthread+0x29f/0x340
> ret_from_fork+0x1f/0x30
> 
> cc: <stable@vger.kernel.org>
> Reported-by: Zhang Yi <yizhan@redhat.com>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>   drivers/gpu/drm/drm_client_modeset.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index a4a62aa99984..5d4703b4648a 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -867,6 +867,9 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>   				break;
>   			}
>   
> +			if (modeset->mode)
> +				kfree(modeset->mode);

kfree() does a NULL-pointer test. So it can be removed here. With that 
changed:

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> +				
>   			modeset->mode = drm_mode_duplicate(dev, mode);
>   			drm_connector_get(connector);
>   			modeset->connectors[modeset->num_connectors++] = connector;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index a4a62aa99984..5d4703b4648a 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -867,6 +867,9 @@  int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
 				break;
 			}
 
+			if (modeset->mode)
+				kfree(modeset->mode);
+				
 			modeset->mode = drm_mode_duplicate(dev, mode);
 			drm_connector_get(connector);
 			modeset->connectors[modeset->num_connectors++] = connector;