Re: [PATCH V2] IB/uverbs: Fix race between uverbs_close and remove_one

On 3/8/2016 12:54 PM, Devesh Sharma wrote:

quoted

What is wrong with simply this:

--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c

@@ -962,9 +962,9 @@ static int ib_uverbs_close(struct inode *inode, struct file *filp)
                 list_del(&file->list);
                 file->is_closed = 1;
         }
-       mutex_unlock(&file->device->lists_mutex);
         if (ucontext)
                 ib_uverbs_cleanup_ucontext(file, ucontext);
+       mutex_unlock(&file->device->lists_mutex);

??

There is following comment about list_mutex in uverbs_main.c around
line number 1200:
/* 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).
  */

Correct.

In addition it's very *bad/incorrect* to call ib_uverbs_cleanup_ucontext under the list mutex as it will prevent parallel cleanups of different contexts, this may end up with softlock ups as cleanup may involve FW commands and may take time.

The correct solution is as I described in my comments to V2, need V3 with the fixes.




--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html