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

[Yishai Hadas <yishaih-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

On 3/8/2016 12:54 PM, Devesh Sharma wrote:
>> 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