[PATCH] RDMA/qedr: Fix potential memory leak in __qedr_alloc_mr()

[PATCH] RDMA/qedr: Fix potential memory leak in __qedr_alloc_mr()

[Jianglei Nie]

__qedr_alloc_mr() allocates a memory chunk for "mr->info.pbl_table" with
init_mr_info(). When rdma_alloc_tid() and rdma_register_tid() fail, "mr"
is released while "mr->info.pbl_table" is not released, which will lead
to a memory leak.

We should release the "mr->info.pbl_table" with qedr_free_pbl() when error
occurs to fix the memory leak.

Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
 drivers/infiniband/hw/qedr/verbs.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 03ed7c0fae50..d745ce9dc88a 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -3084,7 +3084,7 @@ static struct qedr_mr *__qedr_alloc_mr(struct ib_pd *ibpd,
 		else
 			DP_ERR(dev, "roce alloc tid returned error %d\n", rc);
 
-		goto err0;
+		goto err1;
 	}
 
 	/* Index only, 18 bit long, lkey = itid << 8 | key */
@@ -3108,7 +3108,7 @@ static struct qedr_mr *__qedr_alloc_mr(struct ib_pd *ibpd,
 	rc = dev->ops->rdma_register_tid(dev->rdma_ctx, &mr->hw_mr);
 	if (rc) {
 		DP_ERR(dev, "roce register tid returned an error %d\n", rc);
-		goto err1;
+		goto err2;
 	}
 
 	mr->ibmr.lkey = mr->hw_mr.itid << 8 | mr->hw_mr.key;
@@ -3117,8 +3117,10 @@ static struct qedr_mr *__qedr_alloc_mr(struct ib_pd *ibpd,
 	DP_DEBUG(dev, QEDR_MSG_MR, "alloc frmr: %x\n", mr->ibmr.lkey);
 	return mr;
 
-err1:
+err2:
 	dev->ops->rdma_free_tid(dev->rdma_ctx, mr->hw_mr.itid);
+err1:
+	qedr_free_pbl(dev, &mr->info.pbl_info, mr->info.pbl_table);
 err0:
 	kfree(mr);
 	return ERR_PTR(rc);
-- 
2.25.1

RE: [EXT] [PATCH] RDMA/qedr: Fix potential memory leak in __qedr_alloc_mr()

[Michal Kalderon]

> From: Jianglei Nie <niejianglei2021@163.com>
> Sent: Thursday, July 14, 2022 9:15 AM
> ----------------------------------------------------------------------
> __qedr_alloc_mr() allocates a memory chunk for "mr->info.pbl_table" with
> init_mr_info(). When rdma_alloc_tid() and rdma_register_tid() fail, "mr"
> is released while "mr->info.pbl_table" is not released, which will lead
> to a memory leak.
> 
> We should release the "mr->info.pbl_table" with qedr_free_pbl() when
> error
> occurs to fix the memory leak.
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>  drivers/infiniband/hw/qedr/verbs.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/qedr/verbs.c
> b/drivers/infiniband/hw/qedr/verbs.c
> index 03ed7c0fae50..d745ce9dc88a 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -3084,7 +3084,7 @@ static struct qedr_mr *__qedr_alloc_mr(struct
> ib_pd *ibpd,
>  		else
>  			DP_ERR(dev, "roce alloc tid returned error %d\n", rc);
> 
> -		goto err0;
> +		goto err1;
>  	}
> 
>  	/* Index only, 18 bit long, lkey = itid << 8 | key */
> @@ -3108,7 +3108,7 @@ static struct qedr_mr *__qedr_alloc_mr(struct
> ib_pd *ibpd,
>  	rc = dev->ops->rdma_register_tid(dev->rdma_ctx, &mr->hw_mr);
>  	if (rc) {
>  		DP_ERR(dev, "roce register tid returned an error %d\n", rc);
> -		goto err1;
> +		goto err2;
>  	}
> 
>  	mr->ibmr.lkey = mr->hw_mr.itid << 8 | mr->hw_mr.key;
> @@ -3117,8 +3117,10 @@ static struct qedr_mr *__qedr_alloc_mr(struct
> ib_pd *ibpd,
>  	DP_DEBUG(dev, QEDR_MSG_MR, "alloc frmr: %x\n", mr-
> >ibmr.lkey);
>  	return mr;
> 
> -err1:
> +err2:
>  	dev->ops->rdma_free_tid(dev->rdma_ctx, mr->hw_mr.itid);
> +err1:
> +	qedr_free_pbl(dev, &mr->info.pbl_info, mr->info.pbl_table);
>  err0:
>  	kfree(mr);
>  	return ERR_PTR(rc);
> --
> 2.25.1

Thanks, 

Acked-by: Michal Kalderon <michal.kalderon@marvell.com>

Re: [PATCH] RDMA/qedr: Fix potential memory leak in __qedr_alloc_mr()

[Leon Romanovsky]

On Thu, Jul 14, 2022 at 02:15:05PM +0800, Jianglei Nie wrote:
> __qedr_alloc_mr() allocates a memory chunk for "mr->info.pbl_table" with
> init_mr_info(). When rdma_alloc_tid() and rdma_register_tid() fail, "mr"
> is released while "mr->info.pbl_table" is not released, which will lead
> to a memory leak.
> 
> We should release the "mr->info.pbl_table" with qedr_free_pbl() when error
> occurs to fix the memory leak.
> 
> Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
> ---
>  drivers/infiniband/hw/qedr/verbs.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 

Added fixes line.
Fixes: e0290cce6ac0 ("qedr: Add support for memory registeration verbs")

Thanks, applied.