Thread (1 message) 1 message, 1 author, 2012-09-01

Re: [PATCH 1/5] ib_srp: free memory correctly in srp_free_iu()

From: Bart Van Assche <hidden>
Date: 2012-09-01 07:32:37

On 08/31/12 20:00, dongsu.park-EIkl63zCoXaH+58JC4qpiA@public.gmane.org wrote:
quoted hunk
From: Dongsu Park <redacted>

As a potential fix for a race condition in srp_free_iu(),
hold a mutex in srp_free_target_ib() before calling srp_free_iu().

In addition, also clear rx/tx ring after freeing memory.
Both rx_ring[] and tx_ring[] should be reinitialized to NULL,
to prevent other tasks from accessing the freed memory.

Signed-off-by: Dongsu Park <redacted>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 7ae5a00..a0d0ca2 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -291,10 +291,16 @@ static void srp_free_target_ib(struct srp_target_port *target)
 	ib_destroy_cq(target->send_cq);
 	ib_destroy_cq(target->recv_cq);
 
-	for (i = 0; i < SRP_RQ_SIZE; ++i)
+	mutex_lock(&target->mutex);
+	for (i = 0; i < SRP_RQ_SIZE; ++i) {
 		srp_free_iu(target->srp_host, target->rx_ring[i]);
-	for (i = 0; i < SRP_SQ_SIZE; ++i)
+		target->rx_ring[i] = NULL;
+	}
+	for (i = 0; i < SRP_SQ_SIZE; ++i) {
 		srp_free_iu(target->srp_host, target->tx_ring[i]);
+		target->tx_ring[i] = NULL;
+	}
+	mutex_unlock(&target->mutex);
 }
 
 static void srp_path_rec_completion(int status,
It would be interesting if you could provide a little more background
information about this patch. srp_remove_target() waits until all users
of rx_ring[] and tx_ring[] have finished before invoking
srp_free_target_ib(). Or at least, that's how it should work. Did you
come up with this patch after source reading or does this patch address
an issue you hit during testing ? In the last case we have to dig deeper
and we have to find out why it's possible that rx_ring[] and/or
tx_ring[] were accessed while srp_free_target_ib() was in progress.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help