[PATCH] librdmacm: Fix verbs leak due to reentrancy issue

[PATCH] librdmacm: Fix verbs leak due to reentrancy issue

[shamir rabinovitch]

Sean,

I was out of work so I tested the patch "librdmacm: lazy initialization
for ib devices" today. The patch works fine for my test case. The only
concern I have is that I think the patch might leak verbs context. I
think that any call to ucma_init_device must be done with mutex locked.
Please correct me if I am wrong.

Thanks, Shamir


Signed-off-by: shamir rabinovitch <shamir.rabinovitch-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 src/cma.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/cma.c b/src/cma.c
index 0dc229e..42769cf 100644
--- a/src/cma.c
+++ b/src/cma.c
@@ -416,10 +416,13 @@ static int ucma_get_device(struct cma_id_private *id_priv, uint64_t guid)
 
 	return ERR(ENODEV);
 match:
-	if ((ret = ucma_init_device(cma_dev)))
+	pthread_mutex_lock(&mut);
+
+	if ((ret = ucma_init_device(cma_dev))) {
+		pthread_mutex_unlock(&mut);
 		return ret;
+	}
 
-	pthread_mutex_lock(&mut);
 	if (!cma_dev->refcnt++) {
 		cma_dev->pd = ibv_alloc_pd(cma_dev->verbs);
 		if (!cma_dev->pd) {
-- 
1.7.9.5

--
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

RE: [PATCH] librdmacm: Fix verbs leak due to reentrancy issue

[Hefty, Sean]

> I was out of work so I tested the patch "librdmacm: lazy initialization
> for ib devices" today. The patch works fine for my test case. The only
> concern I have is that I think the patch might leak verbs context. I
> think that any call to ucma_init_device must be done with mutex locked.
> Please correct me if I am wrong.

I think you're correct; the lock needs to move up.  I'll apply your fix.  Thanks!
--
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

Re: [PATCH] librdmacm: Fix verbs leak due to reentrancy issue

[Shamir Rabinovith]

On Tue, Apr 22, 2014 at 06:35:45PM +0000, Hefty, Sean wrote:
> > I was out of work so I tested the patch "librdmacm: lazy initialization
> > for ib devices" today. The patch works fine for my test case. The only
> > concern I have is that I think the patch might leak verbs context. I
> > think that any call to ucma_init_device must be done with mutex locked.
> > Please correct me if I am wrong.
> 
> I think you're correct; the lock needs to move up.  I'll apply your fix.  Thanks!

librdmacm git (git://git.openfabrics.org/~shefty/librdmacm.git)
currently only has the lazy initialization patch (23ffef0 librdmacm:
Support lazy initialization) but this patch is missing. can you please
add this fix to the git if you approve it? thanks.
--
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

RE: [PATCH] librdmacm: Fix verbs leak due to reentrancy issue

[Hefty, Sean]

> librdmacm git (git://git.openfabrics.org/~shefty/librdmacm.git)
> currently only has the lazy initialization patch (23ffef0 librdmacm:
> Support lazy initialization) but this patch is missing. can you please
> add this fix to the git if you approve it? thanks.

I pushed the fix upstream.  Thanks!
--
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