[PATCH] dma-buf: fix dma_buf_export init order

[Charan Teja Kalla]

Thanks Christian for this nice cleanup!!

On 12/6/2022 8:42 PM, Christian König wrote:
> @@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  	if (!try_module_get(exp_info->owner))
>  		return ERR_PTR(-ENOENT);
>  
> +	file = dma_buf_getfile(exp_info->size, exp_info->flags);
> +	if (IS_ERR(file)) {
> +		ret = PTR_ERR(file);
> +		goto err_module;
> +	}
> +
> +	if (!exp_info->resv)
> +		alloc_size += sizeof(struct dma_resv);
> +	else
> +		/* prevent &dma_buf[1] == dma_buf->resv */
> +		alloc_size += 1;
>  	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
>  	if (!dmabuf) {
>  		ret = -ENOMEM;
> -		goto err_module;
> +		goto err_file;
>  	}
>  
>  	dmabuf->priv = exp_info->priv;
> @@ -653,44 +656,36 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  	init_waitqueue_head(&dmabuf->poll);
>  	dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll;
>  	dmabuf->cb_in.active = dmabuf->cb_out.active = 0;
> +	mutex_init(&dmabuf->lock);
> +	INIT_LIST_HEAD(&dmabuf->attachments);
>  
>  	if (!resv) {
> -		resv = (struct dma_resv *)&dmabuf[1];
> -		dma_resv_init(resv);
> +		dmabuf->resv = (struct dma_resv *)&dmabuf[1];
> +		dma_resv_init(dmabuf->resv);
> +	} else {
> +		dmabuf->resv = resv;
>  	}
> -	dmabuf->resv = resv;
>  
> -	file = dma_buf_getfile(dmabuf, exp_info->flags);
> -	if (IS_ERR(file)) {
> -		ret = PTR_ERR(file);
> +	ret = dma_buf_stats_setup(dmabuf, file);
> +	if (ret)
>  		goto err_dmabuf;
> -	}
>  
> +	file->private_data = dmabuf;
> +	file->f_path.dentry->d_fsdata = dmabuf;
>  	dmabuf->file = file;
>  
> -	mutex_init(&dmabuf->lock);
> -	INIT_LIST_HEAD(&dmabuf->attachments);
> -
>  	mutex_lock(&db_list.lock);
>  	list_add(&dmabuf->list_node, &db_list.head);
>  	mutex_unlock(&db_list.lock);
>  
> -	ret = dma_buf_stats_setup(dmabuf);
> -	if (ret)
> -		goto err_sysfs;
> -
>  	return dmabuf;
>  
> -err_sysfs:
> -	/*
> -	 * Set file->f_path.dentry->d_fsdata to NULL so that when
> -	 * dma_buf_release() gets invoked by dentry_ops, it exits
> -	 * early before calling the release() dma_buf op.
> -	 */
> -	file->f_path.dentry->d_fsdata = NULL;
> -	fput(file);
>  err_dmabuf:
> +	if (!resv)
> +		dma_resv_fini(dmabuf->resv);
>  	kfree(dmabuf);
> +err_file:
> +	fput(file);

This fput() still endup in calling the dma_buf_file_release() where
there are no checks before accessing the file->private_data(=dmabuf)
which can result in null pointer access. Am I missing something trivial?

>  err_module:
>  	module_put(exp_info->owner);
>  	return ERR_PTR(ret);
> -- 2.34.1