[PATCH RFC/RFT 5/9] drm/vc4: hdmi: drop separate CEC unregistration

[Maxime Ripard]

On Wed, Dec 25, 2024 at 01:10:13AM +0200, Dmitry Baryshkov wrote:
> CEC adapter functions are protected by drm_dev_enter() / _exit(), so it
> is safe to keep CEC registered until DRM device is being torn down. Drop
> custom CEC unsregistration code and rely on the DRM core to unregister
> CEC adapter.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++++++------------------------------------
>  1 file changed, 7 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 3086c2ad3bb2e8fafdc1f37ba985aa5785d49f9a..7b39000369e50f5368384575aa81919df5a7133c 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -2752,13 +2752,6 @@ static const struct cec_adap_ops vc4_hdmi_cec_adap_ops = {
>  	.adap_transmit = vc4_hdmi_cec_adap_transmit,
>  };
>  
> -static void vc4_hdmi_cec_release(void *ptr)
> -{
> -	struct drm_connector *connector = ptr;
> -
> -	drm_connector_cec_unregister(connector);
> -}
> -
>  static int vc4_hdmi_cec_init(struct drm_connector *connector)
>  {
>  	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> @@ -2796,51 +2789,19 @@ static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
>  {
>  	struct platform_device *pdev = vc4_hdmi->pdev;
>  	struct device *dev = &pdev->dev;
> -	int ret;
>  
>  	if (!of_property_present(dev->of_node, "interrupts")) {
>  		dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
>  		return 0;
>  	}
>  
> -	ret = drm_connector_hdmi_cec_adapter_register(&vc4_hdmi->connector,
> -						      &vc4_hdmi_cec_adap_ops,
> -						      vc4_hdmi->variant->card_name,
> -						      1,
> -						      vc4_hdmi_cec_init,
> -						      NULL,
> -						      &pdev->dev);
> -	if (ret < 0)
> -		return ret;
> -
> -	/*
> -	 * NOTE: Strictly speaking, we should probably use a DRM-managed
> -	 * registration there to avoid removing the CEC adapter by the
> -	 * time the DRM driver doesn't have any user anymore.
> -	 *
> -	 * However, the CEC framework already cleans up the CEC adapter
> -	 * only when the last user has closed its file descriptor, so we
> -	 * don't need to handle it in DRM.
> -	 *
> -	 * By the time the device-managed hook is executed, we will give
> -	 * up our reference to the CEC adapter and therefore don't
> -	 * really care when it's actually freed.
> -	 *
> -	 * There's still a problematic sequence: if we unregister our
> -	 * CEC adapter, but the userspace keeps a handle on the CEC
> -	 * adapter but not the DRM device for some reason. In such a
> -	 * case, our vc4_hdmi structure will be freed, but the
> -	 * cec_adapter structure will have a dangling pointer to what
> -	 * used to be our HDMI controller. If we get a CEC call at that
> -	 * moment, we could end up with a use-after-free. Fortunately,
> -	 * the CEC framework already handles this too, by calling
> -	 * cec_is_registered() in cec_ioctl() and cec_poll().
> -	 */

I'd really like to keep that comment, somewhere.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250107/1707fc3f/attachment.sig>