From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
The alloc_commit callback makes the uobj visible to other threads,
and it does so using a 'move' semantic of the uobj kref on the stack
into the public storage (eg the IDR, uobject list and file_private_data)
Once this is done another thread could start up and trigger deletion
of the kref. Fortunately cleanup_rwsem happens to prevent this from
being a bug, but that is a fantastically unclear side effect.
Re-organize things so that alloc_commit is that last thing to touch
the uobj, get rid of the sneaky implicit dependency on cleanup_rwsem,
and add a comment reminding that uobj is no longer kref'd after
alloc_commit.
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
---
drivers/infiniband/core/rdma_core.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index c63583dbc6b9ba..afa03d2f68260d 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -498,24 +498,41 @@ int rdma_explicit_destroy(struct ib_uobject *uobject)
static void alloc_commit_idr_uobject(struct ib_uobject *uobj)
{
- spin_lock(&uobj->ufile->idr_lock);
+ struct ib_uverbs_file *ufile = uobj->ufile;
+
+ spin_lock(&ufile->idr_lock);
/*
* We already allocated this IDR with a NULL object, so
* this shouldn't fail.
+ *
+ * NOTE: Once we set the IDR we loose ownership of our kref on uobj.
+ * It will be put by remove_commit_idr_uobject()
*/
- WARN_ON(idr_replace(&uobj->ufile->idr, uobj, uobj->id));
- spin_unlock(&uobj->ufile->idr_lock);
+ WARN_ON(idr_replace(&ufile->idr, uobj, uobj->id));
+ spin_unlock(&ufile->idr_lock);
}
static void alloc_commit_fd_uobject(struct ib_uobject *uobj)
{
- fd_install(uobj->id, uobj->object);
+ int fd = uobj->id;
+
/* This shouldn't be used anymore. Use the file object instead */
uobj->id = 0;
+
/* Get another reference as we export this to the fops */
uverbs_uobject_get(uobj);
+
+ /*
+ * NOTE: Once we install the file we loose ownership of our kref on
+ * uobj. It will be put by uverbs_close_fd()
+ */
+ fd_install(fd, uobj->object);
}
+/*
+ * In all cases rdma_alloc_commit_uobject() consumes the kref to uobj and the
+ * caller can no longer assume uobj is valid.
+ */
int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
{
struct ib_uverbs_file *ufile = uobj->ufile;
@@ -541,6 +558,7 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj)
list_add(&uobj->list, &ufile->uobjects);
mutex_unlock(&ufile->uobjects_lock);
+ /* alloc_commit consumes the uobj kref */
uobj->type->type_class->alloc_commit(uobj);
up_read(&ufile->cleanup_rwsem);
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
