Re: [PATCH 3/3] dma-buf: restore args on failure of dma_buf_mmap

[Sumit Semwal <sumit.semwal@linaro.org>]

Hi John,

On Thursday 07 February 2013 05:33 AM, John Sheu wrote:
> From: John Sheu <sheu@chromium.org>
>
> Callers to dma_buf_mmap expect to fput() the vma struct's vm_file
> themselves on failure.  Not restoring the struct's data on failure
> causes a double-decrement of the vm_file's refcount.
Thanks for your patch; could you please re-send it to the correct, 
relevant lists and me (as the maintainer of dma-buf) rather than just to 
linux-media ml?

I just chanced to see this patch, otherwise it could easily have slipped 
past me (and other interested parties).

You could run scripts/get_maintainer.pl on your patch to find out the 
right lists / email IDs to CC.

Thanks and best regards,
~Sumit.
>
> Signed-off-by: John Sheu <sheu@google.com>
> ---
>   drivers/base/dma-buf.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index a3f79c4..01daf9c 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -446,6 +446,9 @@ EXPORT_SYMBOL_GPL(dma_buf_kunmap);
>   int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
>   		 unsigned long pgoff)
>   {
> +	struct file *oldfile;
> +	int ret;
> +
>   	if (WARN_ON(!dmabuf || !vma))
>   		return -EINVAL;
>
> @@ -459,14 +462,21 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
>   		return -EINVAL;
>
>   	/* readjust the vma */
> -	if (vma->vm_file)
> -		fput(vma->vm_file);
> -
> +	oldfile = vma->vm_file;
>   	vma->vm_file = get_file(dmabuf->file);
>
>   	vma->vm_pgoff = pgoff;
>
> -	return dmabuf->ops->mmap(dmabuf, vma);
> +	ret = dmabuf->ops->mmap(dmabuf, vma);
> +	if (ret) {
> +		/* restore old parameters on failure */
> +		vma->vm_file = oldfile;
> +		fput(dmabuf->file);
> +	} else {
> +		if (oldfile)
> +			fput(oldfile);
> +	}
> +	return ret;
>   }
>   EXPORT_SYMBOL_GPL(dma_buf_mmap);
>
>