linux-kernel - Re: [PATCH 2/3] drm/exynos: Convert to drmm_mode_config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAQKjZNCpK4rq6DFUtiQ2rxCeb_34Mp54quVto+9LRJMH3=ZhQ@mail.gmail.com>
Date: Mon, 10 Nov 2025 14:22:01 +0900
From: Inki Dae <daeinki@...il.com>
To: Hoyoung Lee <hy_fifty.lee@...sung.com>
Cc: Seung-Woo Kim <sw0312.kim@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org, 
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] drm/exynos: Convert to drmm_mode_config_init() and
 drop manual cleanup

2025년 9월 29일 (월) 오후 1:54, Hoyoung Lee <hy_fifty.lee@...sung.com>님이 작성:
>
> Switch mode-config initialization to drmm_mode_config_init() so that the
> lifetime is tied to drm_device. Remove explicit drm_mode_config_cleanup()
> from error and unbind paths since cleanup is now managed by DRM.
>
> No functional change intended.
>
> Signed-off-by: Hoyoung Lee <hy_fifty.lee@...sung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 6cc7bf77bcac..1aea71778ab1 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -257,7 +257,7 @@ static int exynos_drm_bind(struct device *dev)
>         dev_set_drvdata(dev, drm);
>         drm->dev_private = (void *)private;
>
> -       drm_mode_config_init(drm);
> +       drmm_mode_config_init(drm);
>
>         exynos_drm_mode_config_init(drm);
>
> @@ -297,7 +297,6 @@ static int exynos_drm_bind(struct device *dev)
>  err_unbind_all:
>         component_unbind_all(drm->dev, drm);
>  err_mode_config_cleanup:
> -       drm_mode_config_cleanup(drm);

In the current implementation, there is a potential dereference issue
because the private object may be freed before to_dma_dev(dev) is
called.
When drmm_mode_config_init() is invoked, it registers
drm_mode_config_cleanup() as a managed action. This means that the
cleanup function will be automatically executed later when
drm_dev_put() is called.

The problem arises when drm_dev_put() is called without explicitly
invoking drm_mode_config_cleanup() first, as in the original code. In
that case, the managed cleanup is performed later, which allows
to_dma_dev(dev) to be called after the private object has already been
released.

For reference, the following sequence may occur internally when
drm_mode_config_cleanup() is executed:
1. drm_mode_config_cleanup() is called.
2. During the cleanup of FBs, planes, CRTCs, encoders, and connectors,
framebuffers or GEM objects may be released.
3. At this point, Exynos-specific code could invoke to_dma_dev(dev).

Therefore, the private object must remain valid until
drm_mode_config_cleanup() completes.
It would be safer to adjust the code so that kfree(private) is
performed after drm_dev_put(drm) to ensure the private data remains
available during cleanup.

Thanks,
Inki Dae

>         exynos_drm_cleanup_dma(drm);
>         kfree(private);
>         dev_set_drvdata(dev, NULL);
> @@ -317,7 +316,6 @@ static void exynos_drm_unbind(struct device *dev)
>         drm_atomic_helper_shutdown(drm);
>
>         component_unbind_all(drm->dev, drm);
> -       drm_mode_config_cleanup(drm);

Ditto.

>         exynos_drm_cleanup_dma(drm);
>
>         kfree(drm->dev_private);
> --
> 2.34.1
>
>