Re: [PATCH 07/15] drm/tidss: remove dependency on DRM simple helpers - sashiko-bot

From: sashiko-bot@kernel.org
To: "Diogo Silva" <diogompaissilva@gmail.com>
Cc: linux-amlogic@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	wsa+renesas@sang-engineering.com, imx@lists.linux.dev,
	Frank.Li@kernel.org, neil.armstrong@linaro.org
Subject: Re: [PATCH 07/15] drm/tidss: remove dependency on DRM simple helpers
Date: Sat, 18 Jul 2026 23:54:39 +0000	[thread overview]
Message-ID: <20260718235440.5664F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260719-drm_simple_encoder_init-v1-7-a78c509e3062@gmail.com>

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

Pre-existing issues:
- [High] Use-After-Free during teardown due to object lifetime mismatch between `drm_bridge` and `drm_encoder`.
--

--- Patch [7]: [PATCH 07/15] drm/tidss: remove dependency on DRM simple helpers ---
commit e6abfa8d59d485697fcfe2a3a199e7f378a7e23a
Author: Diogo Silva <diogompaissilva@gmail.com>

drm/tidss: remove dependency on DRM simple helpers

The patch removes the deprecated simple KMS helpers from the tidss driver.
It replaces drm_simple_encoder_init() with a direct call to
drm_encoder_init(), providing driver-local drm_encoder_funcs to handle
cleanup.

> diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
> index 698f8d964ca00..10dbcc6cdf6a8 100644
> --- a/drivers/gpu/drm/tidss/tidss_encoder.c
> +++ b/drivers/gpu/drm/tidss/tidss_encoder.c
[ ... ]
> @@ -81,6 +81,10 @@ static const struct drm_bridge_funcs tidss_bridge_funcs = {
>  	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
>  };
>  
> +static const struct drm_encoder_funcs tidss_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
>  int tidss_encoder_create(struct tidss_device *tidss,
>  			 struct drm_bridge *next_bridge,
>  			 u32 encoder_type, u32 possible_crtcs)
> @@ -95,8 +99,8 @@ int tidss_encoder_create(struct tidss_device *tidss,
>  	if (IS_ERR(t_enc))
>  		return PTR_ERR(t_enc);
>  
> -	ret = drm_simple_encoder_init(&tidss->ddev, &t_enc->encoder,
> -				      encoder_type);
> +	ret = drm_encoder_init(&tidss->ddev, &t_enc->encoder,
> +			       &tidss_encoder_funcs, encoder_type, NULL);

[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free during
teardown due to mismatched object lifetimes?

In tidss_encoder_create(), the t_enc structure is allocated via
devm_drm_bridge_alloc(), which ties the memory's lifetime to the bridge's
kref.

Then the embedded encoder is initialized here and its destroy callback is set
to drm_encoder_cleanup() in tidss_encoder_funcs.

If the driver unbinds while a userspace application holds a DRM file
descriptor, devm drops the initial bridge references. Later, when the final
DRM reference is dropped, drm_mode_config_cleanup() calls the encoder's
destroy callback.

Inside drm_encoder_cleanup(), it detaches the bridge. This drops the final
kref and frees the t_enc container. However, drm_encoder_cleanup() then
continues to access the freed encoder memory, causing a use-after-free.

Does this sequence cause memory corruption during teardown?

>  	if (ret)
>  		return ret;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719-drm_simple_encoder_init-v1-0-a78c509e3062@gmail.com?part=7

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