Commit 03c40442 authored by Yishai Hadas's avatar Yishai Hadas Committed by Doug Ledford

IB/uverbs: Fix reference counting usage of event files

Fix the reference counting usage to be handled in the event file
creation/destruction function, instead of being done by the caller.
This is done for both async/non-async event files.

Based on Jason Gunthorpe report at https://www.mail-archive.com/
linux-rdma@vger.kernel.org/msg24680.html:
"The existing code for this is broken, in ib_uverbs_get_context all
the error paths between ib_uverbs_alloc_event_file and the
kref_get(file->ref) are wrong - this will result in fput() which will
call ib_uverbs_event_close, which will try to do kref_put and
ib_unregister_event_handler - which are no longer paired."
Signed-off-by: default avatarYishai Hadas <yishaih@mellanox.com>
Signed-off-by: default avatarShachar Raindel <raindel@mellanox.com>
Reviewed-by: default avatarJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 7dd78647
...@@ -178,6 +178,7 @@ void idr_remove_uobj(struct idr *idp, struct ib_uobject *uobj); ...@@ -178,6 +178,7 @@ void idr_remove_uobj(struct idr *idp, struct ib_uobject *uobj);
struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file, struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file,
int is_async); int is_async);
void ib_uverbs_free_async_event_file(struct ib_uverbs_file *uverbs_file);
struct ib_uverbs_event_file *ib_uverbs_lookup_comp_file(int fd); struct ib_uverbs_event_file *ib_uverbs_lookup_comp_file(int fd);
void ib_uverbs_release_ucq(struct ib_uverbs_file *file, void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
......
...@@ -367,16 +367,6 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file, ...@@ -367,16 +367,6 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
goto err_file; goto err_file;
} }
file->async_file = filp->private_data;
INIT_IB_EVENT_HANDLER(&file->event_handler, file->device->ib_dev,
ib_uverbs_event_handler);
ret = ib_register_event_handler(&file->event_handler);
if (ret)
goto err_file;
kref_get(&file->async_file->ref);
kref_get(&file->ref);
file->ucontext = ucontext; file->ucontext = ucontext;
fd_install(resp.async_fd, filp); fd_install(resp.async_fd, filp);
...@@ -386,6 +376,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file, ...@@ -386,6 +376,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
return in_len; return in_len;
err_file: err_file:
ib_uverbs_free_async_event_file(file);
fput(filp); fput(filp);
err_fd: err_fd:
......
...@@ -406,10 +406,9 @@ static int ib_uverbs_event_close(struct inode *inode, struct file *filp) ...@@ -406,10 +406,9 @@ static int ib_uverbs_event_close(struct inode *inode, struct file *filp)
} }
spin_unlock_irq(&file->lock); spin_unlock_irq(&file->lock);
if (file->is_async) { if (file->is_async)
ib_unregister_event_handler(&file->uverbs_file->event_handler); ib_unregister_event_handler(&file->uverbs_file->event_handler);
kref_put(&file->uverbs_file->ref, ib_uverbs_release_file); kref_put(&file->uverbs_file->ref, ib_uverbs_release_file);
}
kref_put(&file->ref, ib_uverbs_release_event_file); kref_put(&file->ref, ib_uverbs_release_event_file);
return 0; return 0;
...@@ -541,13 +540,20 @@ void ib_uverbs_event_handler(struct ib_event_handler *handler, ...@@ -541,13 +540,20 @@ void ib_uverbs_event_handler(struct ib_event_handler *handler,
NULL, NULL); NULL, NULL);
} }
void ib_uverbs_free_async_event_file(struct ib_uverbs_file *file)
{
kref_put(&file->async_file->ref, ib_uverbs_release_event_file);
file->async_file = NULL;
}
struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file, struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file,
int is_async) int is_async)
{ {
struct ib_uverbs_event_file *ev_file; struct ib_uverbs_event_file *ev_file;
struct file *filp; struct file *filp;
int ret;
ev_file = kmalloc(sizeof *ev_file, GFP_KERNEL); ev_file = kzalloc(sizeof(*ev_file), GFP_KERNEL);
if (!ev_file) if (!ev_file)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
...@@ -556,15 +562,41 @@ struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file, ...@@ -556,15 +562,41 @@ struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file *uverbs_file,
INIT_LIST_HEAD(&ev_file->event_list); INIT_LIST_HEAD(&ev_file->event_list);
init_waitqueue_head(&ev_file->poll_wait); init_waitqueue_head(&ev_file->poll_wait);
ev_file->uverbs_file = uverbs_file; ev_file->uverbs_file = uverbs_file;
kref_get(&ev_file->uverbs_file->ref);
ev_file->async_queue = NULL; ev_file->async_queue = NULL;
ev_file->is_async = is_async;
ev_file->is_closed = 0; ev_file->is_closed = 0;
filp = anon_inode_getfile("[infinibandevent]", &uverbs_event_fops, filp = anon_inode_getfile("[infinibandevent]", &uverbs_event_fops,
ev_file, O_RDONLY); ev_file, O_RDONLY);
if (IS_ERR(filp)) if (IS_ERR(filp))
kfree(ev_file); goto err_put_refs;
if (is_async) {
WARN_ON(uverbs_file->async_file);
uverbs_file->async_file = ev_file;
kref_get(&uverbs_file->async_file->ref);
INIT_IB_EVENT_HANDLER(&uverbs_file->event_handler,
uverbs_file->device->ib_dev,
ib_uverbs_event_handler);
ret = ib_register_event_handler(&uverbs_file->event_handler);
if (ret)
goto err_put_file;
/* At that point async file stuff was fully set */
ev_file->is_async = 1;
}
return filp;
err_put_file:
fput(filp);
kref_put(&uverbs_file->async_file->ref, ib_uverbs_release_event_file);
uverbs_file->async_file = NULL;
return ERR_PTR(ret);
err_put_refs:
kref_put(&ev_file->uverbs_file->ref, ib_uverbs_release_file);
kref_put(&ev_file->ref, ib_uverbs_release_event_file);
return filp; return filp;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment