Skip to content

Commit

Permalink
IB/uverbs: Fix reference counting usage of event files
Browse files Browse the repository at this point in the history
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: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Shachar Raindel <raindel@mellanox.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
  • Loading branch information
Yishai Hadas authored and Doug Ledford committed Aug 30, 2015
1 parent 7dd7864 commit 03c4044
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
1 change: 1 addition & 0 deletions drivers/infiniband/core/uverbs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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);

void ib_uverbs_release_ucq(struct ib_uverbs_file *file,
Expand Down
11 changes: 1 addition & 10 deletions drivers/infiniband/core/uverbs_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,16 +367,6 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *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;

fd_install(resp.async_fd, filp);
Expand All @@ -386,6 +376,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
return in_len;

err_file:
ib_uverbs_free_async_event_file(file);
fput(filp);

err_fd:
Expand Down
44 changes: 38 additions & 6 deletions drivers/infiniband/core/uverbs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,9 @@ static int ib_uverbs_event_close(struct inode *inode, struct file *filp)
}
spin_unlock_irq(&file->lock);

if (file->is_async) {
if (file->is_async)
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);

return 0;
Expand Down Expand Up @@ -541,13 +540,20 @@ void ib_uverbs_event_handler(struct ib_event_handler *handler,
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,
int is_async)
{
struct ib_uverbs_event_file *ev_file;
struct file *filp;
int ret;

ev_file = kmalloc(sizeof *ev_file, GFP_KERNEL);
ev_file = kzalloc(sizeof(*ev_file), GFP_KERNEL);
if (!ev_file)
return ERR_PTR(-ENOMEM);

Expand All @@ -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_waitqueue_head(&ev_file->poll_wait);
ev_file->uverbs_file = uverbs_file;
kref_get(&ev_file->uverbs_file->ref);
ev_file->async_queue = NULL;
ev_file->is_async = is_async;
ev_file->is_closed = 0;

filp = anon_inode_getfile("[infinibandevent]", &uverbs_event_fops,
ev_file, O_RDONLY);
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;
}

Expand Down

0 comments on commit 03c4044

Please sign in to comment.