Re: [PATCH v2 56/78] drm/meson: encoder_hdmi: Switch to atomic_create_state

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

Pre-existing issues:
- [High] Error paths in `meson_encoder_hdmi_probe` fail to remove the bridge from the global `bridge_list` after it has been added, leading to a Use-After-Free (UAF).
--

--- Patch [56]: [PATCH v2 56/78] drm/meson: encoder_hdmi: Switch to atomic_create_state ---
commit ab73d61f7642f9fa6c7c0313f79338a817383c72
Author: Maxime Ripard <[email protected]>

drm/meson: encoder_hdmi: Switch to atomic_create_state

This commit removes the deprecated drm_bridge_funcs.atomic_reset callback
and its drm_atomic_helper_bridge_reset() helper. It switches to the
atomic_create_state callback instead.

> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 55c0601df3c62..0c7a72cb514ae 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -366,7 +366,7 @@ static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
>  	.atomic_check = meson_encoder_hdmi_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_hdmi_probe(struct meson_drm *priv)

[Severity: High]
This isn't a bug introduced by this patch, but does the error handling in
meson_encoder_hdmi_probe() fail to remove the bridge from the global list?

When the bridge is added:

	drm_bridge_add(&meson_encoder_hdmi->bridge);

	meson_encoder_hdmi->priv = priv;

	/* Encoder */
	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
				      DRM_MODE_ENCODER_TMDS);
	if (ret) {
		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
		goto err_put_node;
	}

If an error occurs, the code jumps to err_put_node, which does not call
drm_bridge_remove().

Since devm cleanup will eventually free the memory, does leaving the freed
bridge linked in bridge_list cause a use-after-free when subsequent drivers
call of_drm_find_and_get_bridge()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=56

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic