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

From: sashiko-bot@kernel.org
To: "Diogo Silva" <diogompaissilva@gmail.com>
Cc: Frank.Li@kernel.org, linux-amlogic@lists.infradead.org,
	imx@lists.linux.dev, wsa+renesas@sang-engineering.com,
	dri-devel@lists.freedesktop.org, neil.armstrong@linaro.org
Subject: Re: [PATCH v2 07/15] drm/tidss: remove dependency on DRM simple helpers
Date: Mon, 20 Jul 2026 16:08:33 +0000	[thread overview]
Message-ID: <20260720160833.BD3311F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260720-drm_simple_encoder_init-v2-7-5020b630668a@gmail.com>

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

Pre-existing issues:
- [High] Missing drm_encoder_cleanup() on error paths causes an immediate Use-After-Free during probe failure due to devm/drmm lifecycle mismatch.
--

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

drm/tidss: remove dependency on DRM simple helpers

The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting. This commit
open-codes drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.

> 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
[ ... ]
> @@ -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);
>  	if (ret)
>  		return ret;
>  

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak the
initialized encoder into the global mode_config list on subsequent error
paths?

If drm_bridge_attach() or drm_bridge_connector_init() fails later in this
function, they return an error directly without cleaning up the initialized
DRM encoder:

	ret = drm_bridge_attach(enc, &t_enc->bridge, NULL,
				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
	if (ret) {
		dev_err(tidss->dev, "bridge attach failed: %d\n", ret);
		return ret;
	}

Because t_enc is allocated via devm_drm_bridge_alloc(), devres frees t_enc
as soon as probe fails. The DRM device itself is also devres-managed, so
its cleanup runs after t_enc is freed.

When drm_mode_config_cleanup() iterates over the encoder_list and calls the
.destroy hook on the now-freed t_enc->encoder, will this trigger a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com?part=7

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