Re: [PATCH v11 rdma-next 2/7] RDMA/core: Create mmap database and cookie helper functions

From: Jason Gunthorpe <jgg@ziepe.ca>
To: Michal Kalderon <michal.kalderon@marvell.com>
Cc: mkalderon@marvell.com, aelior@marvell.com, dledford@redhat.com,
	bmt@zurich.ibm.com, galpress@amazon.com, sleybo@amazon.com,
	leon@kernel.org, linux-rdma@vger.kernel.org
Subject: Re: [PATCH v11 rdma-next 2/7] RDMA/core: Create mmap database and cookie helper functions
Date: Thu, 19 Sep 2019 14:14:50 -0300	[thread overview]
Message-ID: <20190919171450.GA4132@ziepe.ca> (raw)
In-Reply-To: <20190905100117.20879-3-michal.kalderon@marvell.com>

On Thu, Sep 05, 2019 at 01:01:12PM +0300, Michal Kalderon wrote:

>  int rdma_user_mmap_io(struct ib_ucontext *ucontext, struct vm_area_struct *vma,
>  		      unsigned long pfn, unsigned long size, pgprot_t prot)
>  {
>  	struct ib_uverbs_file *ufile = ucontext->ufile;
> -	struct rdma_umap_priv *priv;
> +	int ret;
>  
>  	if (!(vma->vm_flags & VM_SHARED))
>  		return -EINVAL;
> @@ -57,17 +114,240 @@ int rdma_user_mmap_io(struct ib_ucontext *ucontext, struct vm_area_struct *vma,
>  		return -EINVAL;
>  	lockdep_assert_held(&ufile->device->disassociate_srcu);
>  
> -	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> -	if (!priv)
> -		return -ENOMEM;
> +	ret = rdma_umap_priv_init(vma, NULL);
> +	if (ret)
> +		return ret;
>  
>  	vma->vm_page_prot = prot;
>  	if (io_remap_pfn_range(vma, vma->vm_start, pfn, size, prot)) {
> -		kfree(priv);
> +		rdma_umap_priv_delete(ufile, vma->vm_private_data);
>  		return -EAGAIN;

This leaves vma->vm_private_data set to the priv after it has been
freed. This does not seem like such a good idea, can it use after
free? It looks like no, but still would not be a bad idea to null it
here.

Jason