| Message ID | 20190821142125.5706-8-yuval.shaia@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Shared PD and MR | expand |
On Wed, Aug 21, 2019 at 05:21:08PM +0300, Yuval Shaia wrote: > From: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> > > The lock/unlock helpers will be used in every import verb. > > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> > Signed-off-by: Shamir Rabinovitch <srabinov7@gmail.com> > drivers/infiniband/core/uverbs.h | 2 + > drivers/infiniband/core/uverbs_cmd.c | 73 +++++++++++++++++++++++++++ > drivers/infiniband/core/uverbs_main.c | 1 + > 3 files changed, 76 insertions(+) > > diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h > index 1e5aeb39f774..cf76336cb460 100644 > +++ b/drivers/infiniband/core/uverbs.h > @@ -163,6 +163,8 @@ struct ib_uverbs_file { > struct page *disassociate_page; > > struct xarray idr; > + > + struct file *filp; > }; > > struct ib_uverbs_event { > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > index 4f42f9732dca..21f0a1a986f4 100644 > +++ b/drivers/infiniband/core/uverbs_cmd.c > @@ -43,6 +43,7 @@ > > #include <rdma/uverbs_types.h> > #include <rdma/uverbs_std_types.h> > +#include <rdma/uverbs_ioctl.h> > #include "rdma_core.h" > > #include "uverbs.h" > @@ -3791,6 +3792,78 @@ static void uverbs_init_attrs_ufile(struct uverbs_attr_bundle *attrs_bundle, > }; > } > > +/* ib_uverbs_import_lock - Function which gathers code that is > + * common in the import verbs. > + * > + * This function guarntee that both source and destination files are > + * protected from race with vfs close. The current file is protected > + * from such race because verb is executed in a system-call context. > + * The other file is protected by 'fget'. This function also ensures > + * that ib_uobject identified by the type & handle is locked for read. > + * > + * Callers of this helper must also call ib_uverbs_import_unlock > + * to undo any locking performed by this helper. > + */ > +static int ib_uverbs_import_lock(struct uverbs_attr_bundle *attrs, This isn't a lock, it is a get > + int fd, u16 type, u32 handle, > + struct ib_uobject **uobj, > + struct file **filep, > + struct ib_uverbs_file **ufile) > +{ > + struct ib_uverbs_file *file = attrs->ufile; > + struct ib_uverbs_device *dev = file->device; > + struct uverbs_attr_bundle fd_attrs; > + struct ib_uverbs_device *fd_dev; > + int ret = 0; > + > + *filep = fget(fd); > + if (!*filep) > + return -EINVAL; > + > + /* check uverbs ops exist */ > + if ((*filep)->f_op != file->filp->f_op) { > + ret = -EINVAL; > + goto file; > + } > + > + *ufile = (*filep)->private_data; > + fd_dev = (*ufile)->device; Most likely all of this should be in the ioctl code as part of some > + /* check that both files belong to same ib_device */ > + if (dev != fd_dev) { > + ret = -EINVAL; > + goto file; > + } > + > + uverbs_init_attrs_ufile(&fd_attrs, *ufile); This makes no sense here > + *uobj = uobj_get_read(type, handle, &fd_attrs); > + if (IS_ERR(*uobj)) { > + ret = -EINVAL; > + goto file; > + } > + > + /* verify ib_object is shareable */ > + if (!(*uobj)->refcnt) { > + ret = -EINVAL; > + goto uobj; > + } > + > + return 0; > +uobj: > + uobj_put_read(*uobj); > +file: > + fput(*filep); > + return ret; > +} Most likely most of this belongs to the ioctl path as some new input attr type 'foreign context' Jason
diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h index 1e5aeb39f774..cf76336cb460 100644 --- a/drivers/infiniband/core/uverbs.h +++ b/drivers/infiniband/core/uverbs.h @@ -163,6 +163,8 @@ struct ib_uverbs_file { struct page *disassociate_page; struct xarray idr; + + struct file *filp; }; struct ib_uverbs_event { diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c index 4f42f9732dca..21f0a1a986f4 100644 --- a/drivers/infiniband/core/uverbs_cmd.c +++ b/drivers/infiniband/core/uverbs_cmd.c @@ -43,6 +43,7 @@ #include <rdma/uverbs_types.h> #include <rdma/uverbs_std_types.h> +#include <rdma/uverbs_ioctl.h> #include "rdma_core.h" #include "uverbs.h" @@ -3791,6 +3792,78 @@ static void uverbs_init_attrs_ufile(struct uverbs_attr_bundle *attrs_bundle, }; } +/* ib_uverbs_import_lock - Function which gathers code that is + * common in the import verbs. + * + * This function guarntee that both source and destination files are + * protected from race with vfs close. The current file is protected + * from such race because verb is executed in a system-call context. + * The other file is protected by 'fget'. This function also ensures + * that ib_uobject identified by the type & handle is locked for read. + * + * Callers of this helper must also call ib_uverbs_import_unlock + * to undo any locking performed by this helper. + */ +static int ib_uverbs_import_lock(struct uverbs_attr_bundle *attrs, + int fd, u16 type, u32 handle, + struct ib_uobject **uobj, + struct file **filep, + struct ib_uverbs_file **ufile) +{ + struct ib_uverbs_file *file = attrs->ufile; + struct ib_uverbs_device *dev = file->device; + struct uverbs_attr_bundle fd_attrs; + struct ib_uverbs_device *fd_dev; + int ret = 0; + + *filep = fget(fd); + if (!*filep) + return -EINVAL; + + /* check uverbs ops exist */ + if ((*filep)->f_op != file->filp->f_op) { + ret = -EINVAL; + goto file; + } + + *ufile = (*filep)->private_data; + fd_dev = (*ufile)->device; + + /* check that both files belong to same ib_device */ + if (dev != fd_dev) { + ret = -EINVAL; + goto file; + } + + uverbs_init_attrs_ufile(&fd_attrs, *ufile); + + *uobj = uobj_get_read(type, handle, &fd_attrs); + if (IS_ERR(*uobj)) { + ret = -EINVAL; + goto file; + } + + /* verify ib_object is shareable */ + if (!(*uobj)->refcnt) { + ret = -EINVAL; + goto uobj; + } + + return 0; +uobj: + uobj_put_read(*uobj); +file: + fput(*filep); + return ret; +} + +static void ib_uverbs_import_unlock(struct ib_uobject *uobj, + struct file *filep) +{ + uobj_put_read(uobj); + fput(filep); +} + /* * Describe the input structs for write(). Some write methods have an input * only struct, most have an input and output. If the struct has an output then diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c index 02b57240176c..e42a9b5c38b2 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -1095,6 +1095,7 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp) mutex_init(&file->umap_lock); INIT_LIST_HEAD(&file->umaps); + file->filp = filp; filp->private_data = file; list_add_tail(&file->list, &dev->uverbs_file_list); mutex_unlock(&dev->lists_mutex);