On Tue, Oct 27, 2020 at 02:11:22PM -0300, Jason Gunthorpe wrote:
> On Tue, Oct 27, 2020 at 01:55:08PM -0300, Jason Gunthorpe wrote:
>
> > diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> > index 3d366cb79cef42..3ae878f3d173d3 100644
> > +++ b/drivers/infiniband/core/rdma_core.c
> > @@ -540,6 +540,9 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
> > if (ret)
> > return ret;
> >
> > + if (why == RDMA_REMOVE_ABORT)
> > + return 0;
> > +
> > ib_rdmacg_uncharge(&uobj->cg_obj, uobj->context->device,
> > RDMACG_RESOURCE_HCA_OBJECT);
> >
> > @@ -727,10 +730,8 @@ void release_ufile_idr_uobject(struct ib_uverbs_file *ufile)
> > *
> > * This is an optimized equivalent to remove_handle_idr_uobject
> > */
> > - xa_for_each(&ufile->idr, id, entry) {
> > - WARN_ON(entry->object);
> > + xa_for_each(&ufile->idr, id, entry)
> > uverbs_uobject_put(entry);
> > - }
>
> Actually this is not a good idea
>
> This one is better:
This causes to many syzkaller bugs, I didn't debug yet.
Thanks
>
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index 3d366cb79cef42..fd012be700ccc2 100644
> --- a/drivers/infiniband/core/rdma_core.c
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -540,6 +540,9 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
> if (ret)
> return ret;
>
> + if (why == RDMA_REMOVE_ABORT)
> + return 0;
> +
> ib_rdmacg_uncharge(&uobj->cg_obj, uobj->context->device,
> RDMACG_RESOURCE_HCA_OBJECT);
>
> @@ -845,11 +848,17 @@ static int __uverbs_cleanup_ufile(struct ib_uverbs_file *ufile,
> * racing with a lookup_get.
> */
> WARN_ON(uverbs_try_lock_object(obj, UVERBS_LOOKUP_WRITE));
> + if (reason == RDMA_REMOVE_DRIVER_FAILURE)
> + obj->object = NULL;
> if (!uverbs_destroy_uobject(obj, reason, &attrs))
> ret = 0;
> else
> atomic_set(&obj->usecnt, 0);
> }
> + if (reason == RDMA_REMOVE_DRIVER_FAILURE) {
> + WARN_ON(!list_empty(&ufile->uobjects));
> + return 0;
> + }
> return ret;
> }
>
> @@ -862,9 +871,6 @@ static int __uverbs_cleanup_ufile(struct ib_uverbs_file *ufile,
> void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,
> enum rdma_remove_reason reason)
> {
> - struct ib_uobject *obj, *next_obj;
> - unsigned long flags;
> -
> down_write(&ufile->hw_destroy_rwsem);
>
> /*
> @@ -875,25 +881,10 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,
> goto done;
>
> while (!list_empty(&ufile->uobjects))
> - if (__uverbs_cleanup_ufile(ufile, reason)) {
> - /*
> - * No entry was cleaned-up successfully during this
> - * iteration. It is a driver bug to fail destruction.
> - */
> - WARN_ON(!list_empty(&ufile->uobjects));
> + if (__uverbs_cleanup_ufile(ufile, reason))
> break;
> - }
> -
> - list_for_each_entry_safe (obj, next_obj, &ufile->uobjects, list) {
> - spin_lock_irqsave(&ufile->uobjects_lock, flags);
> - list_del_init(&obj->list);
> - spin_unlock_irqrestore(&ufile->uobjects_lock, flags);
> - /*
> - * Pairs with the get in rdma_alloc_commit_uobject(), could
> - * destroy uobj.
> - */
> - uverbs_uobject_put(obj);
> - }
> + if (WARN_ON(!list_empty(&ufile->uobjects)))
> + __uverbs_cleanup_ufile(ufile, RDMA_REMOVE_DRIVER_FAILURE);
> ufile_destroy_ucontext(ufile, reason);
>
> done:
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index edfc1d7d3766ca..7e330f4a6d33ff 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -1471,6 +1471,8 @@ enum rdma_remove_reason {
> RDMA_REMOVE_DRIVER_REMOVE,
> /* uobj is being cleaned-up before being committed */
> RDMA_REMOVE_ABORT,
> + /* The driver failed to destroy the uobject and is being disconnected */
> + RDMA_REMOVE_DRIVER_FAILURE,
> };
>
> struct ib_rdmacg_object {
