On Thu, Sep 05, 2019 at 01:01:13PM +0300, Michal Kalderon wrote:
> static int __efa_mmap(struct efa_dev *dev, struct efa_ucontext *ucontext,
> - struct vm_area_struct *vma, u64 key, u64 length)
> + struct vm_area_struct *vma, u64 key, size_t length)
> {
> - struct efa_mmap_entry *entry;
> + struct rdma_user_mmap_entry *rdma_entry;
> + struct efa_user_mmap_entry *entry;
> unsigned long va;
> u64 pfn;
> int err;
>
> - entry = mmap_entry_get(dev, ucontext, key, length);
> - if (!entry) {
> + rdma_entry = rdma_user_mmap_entry_get(&ucontext->ibucontext, key,
> + length, vma);
> + if (!rdma_entry) {
This allocates memory and assigns it to vma->vm_private
> ibdev_dbg(&dev->ibdev, "key[%#llx] does not have valid entry\n",
> key);
> return -EINVAL;
> }
> + entry = to_emmap(rdma_entry);
> + if (entry->length != length) {
> + ibdev_dbg(&dev->ibdev,
> + "key[%#llx] does not have valid length[%#zx] expected[%#zx]\n",
> + key, length, entry->length);
> + err = -EINVAL;
> + goto err;
Now we take an error..
> +err:
> + rdma_user_mmap_entry_put(&ucontext->ibucontext,
> + rdma_entry);
And this leaks the struct rdma_umap_priv
I said this already, but it looks wrong that rdma_umap_priv_init() is
testing vm_private_data. rdma_user_mmap_io should accept the struct
rdma_user_mmap_entry pointer so it can directly and always create the
priv instead of trying to pass allocated memory through the vm_private
The only place that should set vm_private_data is rdma_user_mmap_io()
and once it succeeds the driver must return success back through
file_operations->mmap()
This hidden detail should also be noted in the comment for
rdma_user_mmap_io..
Jason
