On Wed, Aug 21, 2019 at 05:21:08PM +0300, Yuval Shaia wrote:
> From: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
>
> The lock/unlock helpers will be used in every import verb.
>
> Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> Signed-off-by: Shamir Rabinovitch <srabinov7@xxxxxxxxx>
> 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
