lkml.org 
[lkml]   [2016]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.7 058/184] IB/uverbs: Fix race between uverbs_close and remove_one
    Date
    4.7-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

    commit d1e09f304a1d9651c5059ebfeb696dc2effc9b32 upstream.

    Fixes an oops that might happen if uverbs_close races with
    remove_one.

    Both contexts may run ib_uverbs_cleanup_ucontext, it depends
    on the flow.

    Currently, there is no protection for a case that remove_one
    didn't make the cleanup it runs to its end, the underlying
    ib_device was freed then uverbs_close will call
    ib_uverbs_cleanup_ucontext and OOPs.

    Above might happen if uverbs_close deleted the file from the list
    then remove_one didn't find it and runs to its end.

    Fixes to protect against that case by a new cleanup lock so that
    ib_uverbs_cleanup_ucontext will be called always before that
    remove_one is ended.

    Fixes: 35d4a0b63dc0 ("IB/uverbs: Fix race between ib_uverbs_open and remove_one")
    Reported-by: Devesh Sharma <devesh.sharma@broadcom.com>
    Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
    Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
    Signed-off-by: Leon Romanovsky <leon@kernel.org>
    Signed-off-by: Doug Ledford <dledford@redhat.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/infiniband/core/uverbs.h | 1
    drivers/infiniband/core/uverbs_main.c | 37 ++++++++++++++++++++++------------
    2 files changed, 25 insertions(+), 13 deletions(-)

    --- a/drivers/infiniband/core/uverbs.h
    +++ b/drivers/infiniband/core/uverbs.h
    @@ -116,6 +116,7 @@ struct ib_uverbs_event_file {
    struct ib_uverbs_file {
    struct kref ref;
    struct mutex mutex;
    + struct mutex cleanup_mutex; /* protect cleanup */
    struct ib_uverbs_device *device;
    struct ib_ucontext *ucontext;
    struct ib_event_handler event_handler;
    --- a/drivers/infiniband/core/uverbs_main.c
    +++ b/drivers/infiniband/core/uverbs_main.c
    @@ -931,6 +931,7 @@ static int ib_uverbs_open(struct inode *
    file->async_file = NULL;
    kref_init(&file->ref);
    mutex_init(&file->mutex);
    + mutex_init(&file->cleanup_mutex);

    filp->private_data = file;
    kobject_get(&dev->kobj);
    @@ -956,18 +957,20 @@ static int ib_uverbs_close(struct inode
    {
    struct ib_uverbs_file *file = filp->private_data;
    struct ib_uverbs_device *dev = file->device;
    - struct ib_ucontext *ucontext = NULL;
    +
    + mutex_lock(&file->cleanup_mutex);
    + if (file->ucontext) {
    + ib_uverbs_cleanup_ucontext(file, file->ucontext);
    + file->ucontext = NULL;
    + }
    + mutex_unlock(&file->cleanup_mutex);

    mutex_lock(&file->device->lists_mutex);
    - ucontext = file->ucontext;
    - file->ucontext = NULL;
    if (!file->is_closed) {
    list_del(&file->list);
    file->is_closed = 1;
    }
    mutex_unlock(&file->device->lists_mutex);
    - if (ucontext)
    - ib_uverbs_cleanup_ucontext(file, ucontext);

    if (file->async_file)
    kref_put(&file->async_file->ref, ib_uverbs_release_event_file);
    @@ -1181,22 +1184,30 @@ static void ib_uverbs_free_hw_resources(
    mutex_lock(&uverbs_dev->lists_mutex);
    while (!list_empty(&uverbs_dev->uverbs_file_list)) {
    struct ib_ucontext *ucontext;
    -
    file = list_first_entry(&uverbs_dev->uverbs_file_list,
    struct ib_uverbs_file, list);
    file->is_closed = 1;
    - ucontext = file->ucontext;
    list_del(&file->list);
    - file->ucontext = NULL;
    kref_get(&file->ref);
    mutex_unlock(&uverbs_dev->lists_mutex);
    - /* We must release the mutex before going ahead and calling
    - * disassociate_ucontext. disassociate_ucontext might end up
    - * indirectly calling uverbs_close, for example due to freeing
    - * the resources (e.g mmput).
    - */
    +
    ib_uverbs_event_handler(&file->event_handler, &event);
    +
    + mutex_lock(&file->cleanup_mutex);
    + ucontext = file->ucontext;
    + file->ucontext = NULL;
    + mutex_unlock(&file->cleanup_mutex);
    +
    + /* At this point ib_uverbs_close cannot be running
    + * ib_uverbs_cleanup_ucontext
    + */
    if (ucontext) {
    + /* We must release the mutex before going ahead and
    + * calling disassociate_ucontext. disassociate_ucontext
    + * might end up indirectly calling uverbs_close,
    + * for example due to freeing the resources
    + * (e.g mmput).
    + */
    ib_dev->disassociate_ucontext(ucontext);
    ib_uverbs_cleanup_ucontext(file, ucontext);
    }

    \
     
     \ /
      Last update: 2016-09-23 00:03    [W:4.248 / U:0.248 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site