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] [day] [month] [year] [list]
Message-ID: <CAAQKjZM23M3hr6Xqk6_f9whWw+CQB5oyvs=JEkBQPp2H3-ocag@mail.gmail.com>
Date: Thu, 13 Nov 2025 23:33:21 +0900
From: Inki Dae <daeinki@...il.com>
To: 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년 11월 12일 (수) 오후 12:03, <hy_fifty.lee@...sung.com>님이 작성:
>
> > -----Original Message-----
> > From: Inki Dae <daeinki@...il.com>
> > Sent: Monday, November 10, 2025 2:22 PM
> > 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
> > >
> > >
>
> Hi, Inki
> Thanks for the review and for pointing out the to_dma_dev() path
>
> If I understand you correctly, fine with using DRMM, but kfree(priv) should occur after drm_dev_put(drm)
> That would mean releasing the drm_device first and freeing dev_private afterwards.
> Of course, we will also need to adjust the probe() error-unwind (err_free) order accordingly.
> Do you anticipate any side effects from this ordering change? I’d appreciate your thoughts.
>

Yes, that is correct. Also, changing the order of the cleanup steps
should not introduce any issues, because this simply restores the
original sequence that the code previously followed. In fact, the
current patch is, strictly speaking, altering the existing behavior I
think.

Thanks,
Inki Dae

> BRs,
> Hoyoung Lee
>
>