On Sun, Oct 07, 2018 at 01:30:45PM -0600, Jason Gunthorpe wrote:
> On Sun, Oct 07, 2018 at 11:08:15AM +0300, Shamir Rabinovitch wrote:
> > initialize the context field in ib_udata created from uverbs commands
> > and extended commands path.
> >
> > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > drivers/infiniband/core/uverbs.h | 17 ++++++++------
> > drivers/infiniband/core/uverbs_cmd.c | 34 +++++++++++++--------------
> > drivers/infiniband/core/uverbs_main.c | 6 +++--
> > 3 files changed, 31 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
> > index c97935a0c7c6..7c6c2c975125 100644
> > +++ b/drivers/infiniband/core/uverbs.h
> > @@ -55,23 +55,26 @@ static inline void
> > ib_uverbs_init_udata(struct ib_udata *udata,
> > const void __user *ibuf,
> > void __user *obuf,
> > - size_t ilen, size_t olen)
> > + size_t ilen, size_t olen,
> > + struct ib_ucontext *context)
> > {
> > - udata->inbuf = ibuf;
> > - udata->outbuf = obuf;
> > - udata->inlen = ilen;
> > - udata->outlen = olen;
> > + udata->inbuf = ibuf;
> > + udata->outbuf = obuf;
> > + udata->inlen = ilen;
> > + udata->outlen = olen;
> > + udata->context = context;
> > }
> >
> > static inline void
> > ib_uverbs_init_udata_buf_or_null(struct ib_udata *udata,
> > const void __user *ibuf,
> > void __user *obuf,
> > - size_t ilen, size_t olen)
> > + size_t ilen, size_t olen,
> > + struct ib_ucontext *context)
> > {
> > ib_uverbs_init_udata(udata,
> > ilen ? ibuf : NULL, olen ? obuf : NULL,
> > - ilen, olen);
> > + ilen, olen, context);
> > }
> >
> > /*
> > 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.
Have I missed anything?
>
> 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?
>
> Jason
Thanks
