2021-06-29 06:50:42

by Leon Romanovsky

[permalink] [raw]
Subject: [PATCH rdma-rc v3] RDMA/core: Always release restrack object

From: Leon Romanovsky <[email protected]>

Change location of rdma_restrack_del() to fix the bug where
task_struct was acquired but not released, causing to resource leak.

ucma_create_id() {
ucma_alloc_ctx();
rdma_create_user_id() {
rdma_restrack_new();
rdma_restrack_set_name() {
rdma_restrack_attach_task.part.0(); <--- task_struct was gotten
}
}
ucma_destroy_private_ctx() {
ucma_put_ctx();
rdma_destroy_id() {
_destroy_id() <--- id_priv was freed
}
}
}

Fixes: 889d916b6f8a ("RDMA/core: Don't access cm_id after its destruction")
Reported-by: Pavel Skripkin <[email protected]>
Signed-off-by: Leon Romanovsky <[email protected]>
---
---
Changelog:
v3:
* Dropped controversial hunks and updated commit message respectively
v2: https://lore.kernel.org/lkml/e2eed941f912b2068e371fd37f43b8cf5082a0e6.1623129597.git.leonro@nvidia.com
* Added bug report analysis
v1: https://lore.kernel.org/linux-rdma/f72e27d5c82cd9beec7670141afa62786836c569.1622956637.git.leonro@nvidia.com/T/#u
---
drivers/infiniband/core/cma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index ab148a696c0c..6d103c42bbec 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1852,6 +1852,7 @@ static void _destroy_id(struct rdma_id_private *id_priv,
{
cma_cancel_operation(id_priv, state);

+ rdma_restrack_del(&id_priv->res);
if (id_priv->cma_dev) {
if (rdma_cap_ib_cm(id_priv->id.device, 1)) {
if (id_priv->cm_id.ib)
@@ -1861,7 +1862,6 @@ static void _destroy_id(struct rdma_id_private *id_priv,
iw_destroy_cm_id(id_priv->cm_id.iw);
}
cma_leave_mc_groups(id_priv);
- rdma_restrack_del(&id_priv->res);
cma_release_dev(id_priv);
}

--
2.31.1


2021-06-29 23:10:18

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH rdma-rc v3] RDMA/core: Always release restrack object

On Tue, Jun 29, 2021 at 09:49:33AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <[email protected]>
>
> Change location of rdma_restrack_del() to fix the bug where
> task_struct was acquired but not released, causing to resource leak.
>
> ucma_create_id() {
> ucma_alloc_ctx();
> rdma_create_user_id() {
> rdma_restrack_new();
> rdma_restrack_set_name() {
> rdma_restrack_attach_task.part.0(); <--- task_struct was gotten
> }
> }
> ucma_destroy_private_ctx() {
> ucma_put_ctx();
> rdma_destroy_id() {
> _destroy_id() <--- id_priv was freed
> }
> }
> }
>
> Fixes: 889d916b6f8a ("RDMA/core: Don't access cm_id after its destruction")
> Reported-by: Pavel Skripkin <[email protected]>
> Signed-off-by: Leon Romanovsky <[email protected]>
> ---
> ---
> Changelog:
> v3:
> * Dropped controversial hunks and updated commit message respectively
> v2: https://lore.kernel.org/lkml/e2eed941f912b2068e371fd37f43b8cf5082a0e6.1623129597.git.leonro@nvidia.com
> * Added bug report analysis
> v1: https://lore.kernel.org/linux-rdma/f72e27d5c82cd9beec7670141afa62786836c569.1622956637.git.leonro@nvidia.com/T/#u
> ---
> drivers/infiniband/core/cma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-next, thanks

Jason