On Wed, Oct 10, 2018 at 10:47:19AM +0300, Shamir Rabinovitch wrote: > > > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > > > index ef83813aefc2..c464ed65cbbb 100644 > > > +++ b/drivers/infiniband/core/uverbs_cmd.c > > > @@ -100,7 +100,7 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file, > > > ib_uverbs_init_udata(&udata, buf + sizeof(cmd), > > > u64_to_user_ptr(cmd.response) + sizeof(resp), > > > in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr), > > > - out_len - sizeof(resp)); > > > + out_len - sizeof(resp), file->ucontext); > > > > This accesses ignores this comment explaining the mandatory locking: > > > > /* > > * ucontext must be accessed via ib_uverbs_get_ucontext() or with > > * ucontext_lock held > > */ > > struct ib_ucontext *ucontext; > > > > So, no to all of this patch. > > I see that few lines above the call to ib_uverbs_init_udata the function > ib_uverbs_get_context grab the mentioned lock. It is not a lock, it is reading the data > Have I missed anything? No handler can contain 'file->ucontext', must use the accessor or read it from the uobject. > > In most cases ucontext must be set in the udata by the uobj_* > > functions from the uobject they are handling, and never read from the > > file. > > > > The only exception might be the few uobject-less functions... > > If the above is not an issue, do you still want me to modify this patch > to take the ucontext from the uobject fed in to uobj_get_obj_read? It is an issue, and this is the best resolution.. Jason
