[PATCH 32/64] drm/vc4: dsi: Fix the driver structure lifetime

[Thomas Zimmermann]

Am 10.06.22 um 11:28 schrieb Maxime Ripard:
> The vc4_dsi structure is currently allocated through a device-managed
> allocation. This can lead to use-after-free issues however in the unbinding
> path since the DRM entities will stick around, but the underlying structure
> has been freed.
> 
> However, we can't just fix it by using a DRM-managed allocation like we did
> for the other drivers since the DSI case is a bit more intricate.
> 
> Indeed, the structure will be allocated at probe time, when we don't have a
> DRM device yet, to be able to register the DSI bus driver. We will then
> reuse it at bind time to register our KMS entities in the framework.
> 
> In order to work around both constraints, we can use a kref to track the
> users of the structure (DSI host, and KMS), and then put our structure when
> the DSI host will have been unregistered, and through a DRM-managed action
> that will execute once we won't need the KMS entities anymore.
> 
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> ---
>   drivers/gpu/drm/vc4/vc4_dsi.c | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index 10533a2a41b3..282537f27b8e 100644
> --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> @@ -510,6 +510,8 @@ struct vc4_dsi {
>   	struct vc4_encoder encoder;
>   	struct mipi_dsi_host dsi_host;
>   
> +	struct kref kref;
> +
>   	struct platform_device *pdev;
>   
>   	struct drm_bridge *bridge;
> @@ -1479,6 +1481,15 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi)
>   				      dsi->clk_onecell);
>   }
>   
> +static void vc4_dsi_release(struct kref *kref);
> +
> +static void vc4_dsi_put(struct drm_device *drm, void *ptr)
> +{
> +	struct vc4_dsi *dsi = ptr;
> +
> +	kref_put(&dsi->kref, &vc4_dsi_release);
> +}
> +
>   static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
>   {
>   	struct platform_device *pdev = to_platform_device(dev);
> @@ -1488,6 +1499,12 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
>   	dma_cap_mask_t dma_mask;
>   	int ret;
>   
> +	kref_get(&dsi->kref);
> +
> +	ret = drmm_add_action_or_reset(drm, vc4_dsi_put, dsi);
> +	if (ret)
> +		return ret;
> +
>   	dsi->variant = of_device_get_match_data(dev);
>   
>   	INIT_LIST_HEAD(&dsi->bridge_chain);
> @@ -1642,16 +1659,25 @@ static const struct component_ops vc4_dsi_ops = {
>   	.unbind = vc4_dsi_unbind,
>   };
>   
> +static void vc4_dsi_release(struct kref *kref)
> +{
> +	struct vc4_dsi *dsi =
> +		container_of(kref, struct vc4_dsi, kref);
> +
> +	kfree(dsi);
> +}
> +
>   static int vc4_dsi_dev_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct vc4_dsi *dsi;
>   
> -	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> +	dsi = kzalloc(sizeof(*dsi), GFP_KERNEL);
>   	if (!dsi)
>   		return -ENOMEM;
>   	dev_set_drvdata(dev, dsi);
>   
> +	kref_init(&dsi->kref);
>   	dsi->pdev = pdev;
>   	dsi->dsi_host.ops = &vc4_dsi_host_ops;
>   	dsi->dsi_host.dev = dev;
> @@ -1666,6 +1692,7 @@ static int vc4_dsi_dev_remove(struct platform_device *pdev)
>   	struct vc4_dsi *dsi = dev_get_drvdata(dev);
>   
>   	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	kref_put(&dsi->kref, &vc4_dsi_release);

Maybe vc4_dsi_put() ?

>   	return 0;
>   }
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220620/a91c5ad8/attachment.sig>