[PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[Liam Zuiderhoek]

Inline the function drm_simple_encoder_init for meson,
as per the todo list on docs.kernel.org/gpu/todo.html

Signed-off-by: Liam Zuiderhoek <zuiderhoekl@gmail.com>
---
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 13 +++++++++++--
 1 file changed, 11 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..f4100e23dd27 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -44,9 +44,17 @@ 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)
 
+static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
+	.destroy = drm_encoder_cleanup,
+};
+
 static int meson_encoder_hdmi_attach(struct drm_bridge *bridge,
 				     struct drm_encoder *encoder,
 				     enum drm_bridge_attach_flags flags)
@@ -407,8 +415,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

Re: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a duplicate definition of the `drm_simple_encoder_funcs_cleanup` structure, causing a build failure.
--

commit 2bbec9d596493f3ca9a28c3ddb628d5753d83aa2
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.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 55c0601df3c62..f4100e23dd270 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -44,9 +44,17 @@ 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)
>  
> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
> +	.destroy = drm_encoder_cleanup,
> +};

[Severity: Low]
Does this code cause a compiler error? It appears that the static structure
drm_simple_encoder_funcs_cleanup is defined twice in the same file.

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

[PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[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

Re: [PATCH] GPU: drm: meson: inline call to drm_simple_encoder_init()

[sashiko-bot]

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

Pre-existing issues:
- [High] The DRM bridge is added to the global bridge list but is not removed on subsequent error paths, leading to a Use-After-Free.
- [High] The devm-managed encoder is registered to the DRM device, causing a Use-After-Free on unbind if the DRM device outlives the platform device.
--

commit 38b3bb11e881972fb31cc13c9266c01adfa5805b
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.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 55c0601df3c62..0dd3a76ad959e 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 isn't a bug introduced by this patch, but does registering a devres
managed encoder with the DRM device cause a use-after-free during unbind?

The encoder structure is allocated earlier using devres:

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

It is then initialized and registered into the DRM device's mode config list
here. If the driver is unbound and the platform device is destroyed, devres
will free the encoder memory. However, if userspace holds an open DRM file
descriptor, the DRM device outlives the platform device.

When the file descriptor is later closed and the DRM device is finally
destroyed, drm_mode_config_cleanup() will attempt to call the .destroy
callback on the freed encoder memory.

>  	if (ret) {
>  		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
>  		goto err_put_node;

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

Earlier in meson_encoder_hdmi_probe(), the bridge is added to the global
bridge list:

	drm_bridge_add(&meson_encoder_hdmi->bridge);

If drm_encoder_init() fails, we jump to err_put_node, which returns without
calling drm_bridge_remove(). The devres cleanup will then free
meson_encoder_hdmi, leaving a dangling pointer in the DRM core's global
bridge list.

>  	}

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