Re: [PATCH v2 54/78] drm/meson: encoder_cvbs: Switch to atomic_create_state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing drm_bridge_remove() on error paths in meson_encoder_cvbs_probe leads to a leaked bridge, leaked 'next_bridge' reference, and a dangling pointer in the global bridge_list.
--

--- Patch [54]: [PATCH v2 54/78] drm/meson: encoder_cvbs: Switch to atomic_create_state ---
commit 495782f6fcc8d5b1c7e6fd45af9f1be19848e42c
Author: Maxime Ripard <mripard@kernel.org>

drm/meson: encoder_cvbs: Switch to atomic_create_state

This commit switches the meson_encoder_cvbs bridge from the deprecated
atomic_reset callback to the atomic_create_state callback.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> @@ -215,7 +215,7 @@ static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
>  	.atomic_check = meson_encoder_cvbs_atomic_check,
>  	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> -	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
>  };
>  
>  int meson_encoder_cvbs_probe(struct meson_drm *priv)
>  {

[ ... ]

>  	drm_bridge_add(&meson_encoder_cvbs->bridge);
>  
>  	meson_encoder_cvbs->priv = priv;
>  
>  	/* Encoder */
>  	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
>  				      DRM_MODE_ENCODER_TVDAC);
>  	if (ret)
>  		return dev_err_probe(priv->dev, ret,
>  				     "Failed to init CVBS encoder\n");

[Severity: High]
This is a pre-existing issue, but does this error path leak the bridge?

If drm_simple_encoder_init() fails, the function returns directly without
calling drm_bridge_remove(). It looks like this leaves the bridge object
in the global bridge_list.

Could this lead to a use-after-free or kernel oops if the driver module
is unloaded and another subsystem iterates over the list? Should there
be a cleanup path here for drm_bridge_add() and the next_bridge reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-drm-no-more-bridge-reset-v2-0-0a91018bf886@kernel.org?part=54

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