Re: [PATCH v6 04/15] crypto: ti - Fix use-after-free of dev_data on DTHEv2 driver removal

[Herbert Xu <herbert@gondor.apana.org.au>]

On Wed, Sep 30, 2026 at 04:55:22PM +0530, T Pratham wrote:
>
> +struct dthe_data *dthe_get_drvdata(struct device *dev)
> +{
> +	struct dthe_data *dev_data;
> +
> +	spin_lock_bh(&dthe_dev_list.lock);
> +	dev_data = dev_get_drvdata(dev);
> +	if (dev_data && !atomic_fetch_inc(&dev_data->req_refcnt))
> +		reinit_completion(&dev_data->drain_compl);

This is correct but rather baroque.

How about simplifying it by making the drvdata also hold a ref
count.  That way the refcount will only hit zero during the removal
process.  So you don't have to keep reinitialising the completion
object over and over again.

Also refcnt should use kref_t which is easier to use than atomic_t.

+struct dthe_data *dthe_get_drvdata(struct device *dev)
+{
+	struct dthe_data *dev_data;
+
+	spin_lock_bh(&dthe_dev_list.lock);
+	dev_data = dev_get_drvdata(dev);
+	if (dev_data)
+		kref_get(&dev_data->req_refcnt);
 	spin_unlock_bh(&dthe_dev_list.lock);

 	return dev_data;
 }

+void dthe_release_drvdata(struct kref *kref)
+{
+	struct dthe_data dev_data = container_of(kref, struct dthe_data, req_refcnt);
+
+	complete(&dev_data->drain_compl);
+}
+
+void dthe_put_drvdata(struct dthe_data *dev_data)
+{
+	kref_put(&dev_data->req_refcnt, dthe_release_drvdata);
+		complete(&dev_data->drain_compl);
+}
+

> @@ -152,6 +176,10 @@ static int dthe_probe(struct platform_device *pdev)
>  	if (IS_ERR(dev_data->regs))
>  		return PTR_ERR(dev_data->regs);
>  
> +	atomic_set(&dev_data->req_refcnt, 0);

Use kref_init so that drvdata holds a refcount too.

> +	init_completion(&dev_data->drain_compl);
> +	complete(&dev_data->drain_compl);

You can then drop the complete call as the last kref_put will
automatically complete the object.

> @@ -202,10 +230,13 @@ static void dthe_remove(struct platform_device *pdev)
>  
>  	spin_lock_bh(&dthe_dev_list.lock);
>  	list_del(&dev_data->list);
> +	platform_set_drvdata(pdev, NULL);
>  	spin_unlock_bh(&dthe_dev_list.lock);
>  
>  	dthe_unregister_algs();

Just add this here:

+	dthe_put_drvdata(dev_data);

> +	wait_for_completion(&dev_data->drain_compl);

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt