Skip to contentSkip to conversation

ib_uverbs: Fix pages leak when using XRC SRQs

PATCH In review Sébastien Dugué 2 participants 2 messages Apr 7, 2015 – Apr 14, 2015 InfiniBand

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ib_uverbs: Fix pages leak when using XRC SRQs

Hello,

When an application using XRCs abruptly terminates, the mmaped pages
of the CQ buffers are leaked.

This comes from the fact that when resources are released in
ib_uverbs_cleanup_ucontext(), we fail to release the CQs because their refcount is not 0.

When creating an XRC SRQ, we increment the associated CQ refcount.
This refcount is only decremented when the SRQ is released.

Therefore we need to release the SRQs prior to the CQs to make sure
that all references to the CQs are gone before trying to release these.

---
 drivers/infiniband/core/uverbs_main.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
drivers/infiniband/core/uverbs_main.c +11 -11
index 259dcc7..88cce9b 100644
@@ -246,6 +246,17 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		kfree(uqp);
 	}
 
+	list_for_each_entry_safe(uobj, tmp, &context->srq_list, list) {
+		struct ib_srq *srq = uobj->object;
+		struct ib_uevent_object *uevent =
+			container_of(uobj, struct ib_uevent_object, uobject);
+
+		idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
+		ib_destroy_srq(srq);
+		ib_uverbs_release_uevent(file, uevent);
+		kfree(uevent);
+	}
+
 	list_for_each_entry_safe(uobj, tmp, &context->cq_list, list) {
 		struct ib_cq *cq = uobj->object;
 		struct ib_uverbs_event_file *ev_file = cq->cq_context;
@@ -258,17 +269,6 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
 		kfree(ucq);
 	}
 
-	list_for_each_entry_safe(uobj, tmp, &context->srq_list, list) {
-		struct ib_srq *srq = uobj->object;
-		struct ib_uevent_object *uevent =
-			container_of(uobj, struct ib_uevent_object, uobject);
-
-		idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
-		ib_destroy_srq(srq);
-		ib_uverbs_release_uevent(file, uevent);
-		kfree(uevent);
-	}
-
 	list_for_each_entry_safe(uobj, tmp, &context->mr_list, list) {
 		struct ib_mr *mr = uobj->object;
 
Signature · 6 lines
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reviewed-by on v1 1/1
#1 lore raw
Quoted 52 lines · Sébastien Dugué

Hello,

When an application using XRCs abruptly terminates, the mmaped pages
of the CQ buffers are leaked.

This comes from the fact that when resources are released in
ib_uverbs_cleanup_ucontext(), we fail to release the CQs because their refcount is not 0.

When creating an XRC SRQ, we increment the associated CQ refcount.
This refcount is only decremented when the SRQ is released.

Therefore we need to release the SRQs prior to the CQs to make sure
that all references to the CQs are gone before trying to release these.

---
  drivers/infiniband/core/uverbs_main.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)
drivers/infiniband/core/uverbs_main.c +11 -4
index 259dcc7..88cce9b 100644
@@ -246,6 +246,17 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
  		kfree(uqp);
  	}
 
+	list_for_each_entry_safe(uobj, tmp, &context->srq_list, list) {
+		struct ib_srq *srq = uobj->object;
+		struct ib_uevent_object *uevent =
+			container_of(uobj, struct ib_uevent_object, uobject);
+
+		idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
+		ib_destroy_srq(srq);
+		ib_uverbs_release_uevent(file, uevent);
+		kfree(uevent);
+	}
+
  	list_for_each_entry_safe(uobj, tmp, &context->cq_list, list) {
  		struct ib_cq *cq = uobj->object;
  		struct ib_uverbs_event_file *ev_file = cq->cq_context;
@@ -258,17 +269,6 @@ static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file,
  		kfree(ucq);
  	}
 
-	list_for_each_entry_safe(uobj, tmp, &context->srq_list, list) {
-		struct ib_srq *srq = uobj->object;
-		struct ib_uevent_object *uevent =
-			container_of(uobj, struct ib_uevent_object, uobject);

drivers/infiniband/core/uverbs_main.c static int ib_uverbs_cleanup_ucontext(struct ib_uverbs_file *file, · line 274

Quoted 10 lines
drivers/infiniband/core/uverbs_main.c +0 -7
-
-		idr_remove_uobj(&ib_uverbs_srq_idr, uobj);
-		ib_destroy_srq(srq);
-		ib_uverbs_release_uevent(file, uevent);
-		kfree(uevent);
-	}
-
  	list_for_each_entry_safe(uobj, tmp, &context->mr_list, list) {
  		struct ib_mr *mr = uobj->object;
 

Nice catch,

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html