mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()
@ 2026-07-09 20:55 Liam Zuiderhoek
  2026-07-09 21:08 ` sashiko-bot
  2026-07-10  5:50 ` Martin Blumenstingl
  0 siblings, 2 replies; 3+ messages in thread
From: Liam Zuiderhoek @ 2026-07-09 20:55 UTC (permalink / raw)
  To: dri-devel
  Cc: neil.armstrong, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona, khilman, jbrunet, martin.blumenstingl, linux-amlogic,
	linux-arm-kernel, linux-kernel, Liam Zuiderhoek

This commit inlines the function drm_simple_encoder_init for the meson
driver. It replaces the simple encoder init call with drm_encoder_init
and a custom cleanup struct.

Signed-off-by: Liam Zuiderhoek <zuiderhoekl@gmail.com>
---
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 55c0601df3c6..0dd3a76ad959 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -44,6 +44,10 @@ struct meson_encoder_hdmi {
 	struct cec_notifier *cec_notifier;
 };
 
+static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
+	.destroy = drm_encoder_cleanup,
+};
+
 #define bridge_to_meson_encoder_hdmi(x) \
 	container_of(x, struct meson_encoder_hdmi, bridge)
 
@@ -407,8 +411,9 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
 	meson_encoder_hdmi->priv = priv;
 
 	/* Encoder */
-	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
-				      DRM_MODE_ENCODER_TMDS);
+	ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
+			       &drm_simple_encoder_funcs_cleanup,
+			       DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret) {
 		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
 		goto err_put_node;
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()
  2026-07-09 20:55 [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init() Liam Zuiderhoek
@ 2026-07-09 21:08 ` sashiko-bot
  2026-07-10  5:50 ` Martin Blumenstingl
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-09 21:08 UTC (permalink / raw)
  To: Liam Zuiderhoek; +Cc: linux-amlogic, neil.armstrong, dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The commit message claims to inline `drm_simple_encoder_init()` for the meson driver, but it only modifies the HDMI encoder, leaving the deprecated helper in use by other encoders in the same driver.

Pre-existing issues:
- [High] The `meson_encoder_hdmi` structure (which embeds `struct drm_encoder`) is allocated with `devm_drm_bridge_alloc()`, but the encoder is initialized with `drm_encoder_init()`. This creates a mismatch in lifecycles that leads to a use-after-free when the platform device is unbound but userspace still holds a reference to the DRM device.
--

--- Patch [1]: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init() ---
commit bb202202974455054588168b82acfc49ff85b23f
Author: Liam Zuiderhoek <zuiderhoekl@gmail.com>

GPU: drm: meson: inline call to drm_simple_encoder_init()

This commit inlines the function drm_simple_encoder_init for the meson
driver. It replaces the simple encoder init call with drm_encoder_init
and a custom cleanup struct.

> This commit inlines the function drm_simple_encoder_init for the meson
> driver. It replaces the simple encoder init call with drm_encoder_init
> and a custom cleanup struct.

[Severity: Low]
Does this commit miss updating other encoders in the meson driver?

The commit message implies it inlines the function for the meson driver as a
whole, but meson_encoder_cvbs_probe() in meson_encoder_cvbs.c and
meson_encoder_dsi_probe() in meson_encoder_dsi.c still use
drm_simple_encoder_init().

Additionally, is the include for <drm/drm_simple_kms_helper.h> still needed
in meson_encoder_hdmi.c now that the helper has been open-coded?

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 0c7a72cb514ae..70a543358041f 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c

[ ... ]

> @@ -407,8 +411,9 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
>  	meson_encoder_hdmi->priv = priv;
>  
>  	/* Encoder */
> -	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> -				      DRM_MODE_ENCODER_TMDS);
> +	ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> +			       &drm_simple_encoder_funcs_cleanup,
> +			       DRM_MODE_ENCODER_TMDS, NULL);

[Severity: High]
This is a pre-existing issue, but does this allocation lifecycle mismatch
lead to a use-after-free?

Looking at meson_encoder_hdmi_probe(), the structure embedding the encoder is
allocated with devm_drm_bridge_alloc(), tying its lifetime to the platform
device:

meson_encoder_hdmi_probe() {
    ...
    meson_encoder_hdmi = devm_drm_bridge_alloc(priv->dev, ...);
    ...
}

However, the encoder is initialized here with drm_encoder_init(), tying its
cleanup to the DRM device's release.

If the platform device is unbound, devres automatically frees
meson_encoder_hdmi. If a userspace process holds an open file descriptor to
the DRM device during unbind, the DRM device outlives the platform device.

When the userspace process eventually closes the fd, drm_dev_release()
calls drm_mode_config_cleanup(), which iterates over the mode config list
and accesses the freed meson_encoder_hdmi->encoder to call its destroy
callback.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709205538.73157-1-zuiderhoekl@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()
  2026-07-09 20:55 [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init() Liam Zuiderhoek
  2026-07-09 21:08 ` sashiko-bot
@ 2026-07-10  5:50 ` Martin Blumenstingl
  1 sibling, 0 replies; 3+ messages in thread
From: Martin Blumenstingl @ 2026-07-10  5:50 UTC (permalink / raw)
  To: Liam Zuiderhoek
  Cc: dri-devel, neil.armstrong, maarten.lankhorst, mripard,
	tzimmermann, airlied, simona, khilman, jbrunet, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hello,

On Thu, Jul 9, 2026 at 10:56 PM Liam Zuiderhoek <zuiderhoekl@gmail.com> wrote:
>
> This commit inlines the function drm_simple_encoder_init for the meson
> driver. It replaces the simple encoder init call with drm_encoder_init
> and a custom cleanup struct.
>
> Signed-off-by: Liam Zuiderhoek <zuiderhoekl@gmail.com>
What about meson_encoder_dsi and meson_encoder_cvbs?
For all three there's already another series - maybe you can help review it: [0]


Thank you and best regards,
Martin


[0] https://lore.kernel.org/linux-amlogic/20260531073532.8609-1-namanarora029@gmail.com/

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-10  5:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-09 20:55 [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init() Liam Zuiderhoek
2026-07-09 21:08 ` sashiko-bot
2026-07-10  5:50 ` Martin Blumenstingl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox