Re: [PATCH v2] RDMA/uverbs: Free all radix tree nodes in uverbs_destroy_api()

Re: [PATCH v2] RDMA/uverbs: Free all radix tree nodes in uverbs_destroy_api()

[syzbot]

> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

Patch testing via #syz test is temporarily disabled.
See https://github.com/google/syzkaller/issues/8122 for details.

>
>
> syzbot reported a memory leak of radix_tree_node objects allocated
> while registering a siw device:
>
>   radix_tree_node_alloc+0x81/0x1a0 lib/radix-tree.c:276
>   radix_tree_insert+0x140/0x390 lib/radix-tree.c:712
>   uapi_add_elm+0x61/0x140 uverbs_uapi.c:41
>   uverbs_alloc_api+0x85/0xc50 uverbs_uapi.c:659
>   siw_newlink+0x36a/0x3f0 siw_main.c:433
>
> radix_tree_insert() can fail with -ENOMEM after it has already linked
> intermediate nodes into the tree, leaving them with no entries.
> uverbs_destroy_api() only removes populated slots through
> uapi_remove_range(), and radix tree nodes are freed only as a side
> effect of deleting an entry, so these empty nodes are leaked when the
> uapi is destroyed.
>
> Call xa_destroy() after removing the entries so that all remaining
> nodes are freed.
>
> Reported-by: syzbot+d60be66dd00b4490fa96@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d60be66dd00b4490fa96
> Fixes: 9ed3e5f44772 ("IB/uverbs: Build the specs into a radix tree at runtime")
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> Changes in v2:
> - Drop the dummy insert/delete workaround in uapi_add_elm()
> - Call xa_destroy() in uverbs_destroy_api() to free all remaining
>   nodes, as suggested by Jason
> ---
>  drivers/infiniband/core/uverbs_uapi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
> index b67db3790f03..77a4eb494936 100644
> --- a/drivers/infiniband/core/uverbs_uapi.c
> +++ b/drivers/infiniband/core/uverbs_uapi.c
> @@ -621,6 +621,7 @@ void uverbs_destroy_api(struct uverbs_api *uapi)
>  		return;
>  
>  	uapi_remove_range(uapi, 0, U32_MAX);
> +	xa_destroy(&uapi->radix);
>  	kfree(uapi->write_methods);
>  	kfree(uapi);
>  }
> -- 
> 2.43.0
>

[PATCH v2] RDMA/uverbs: Free all radix tree nodes in uverbs_destroy_api()

[Deepanshu Kartikey]

syzbot reported a memory leak of radix_tree_node objects allocated
while registering a siw device:

  radix_tree_node_alloc+0x81/0x1a0 lib/radix-tree.c:276
  radix_tree_insert+0x140/0x390 lib/radix-tree.c:712
  uapi_add_elm+0x61/0x140 uverbs_uapi.c:41
  uverbs_alloc_api+0x85/0xc50 uverbs_uapi.c:659
  siw_newlink+0x36a/0x3f0 siw_main.c:433

radix_tree_insert() can fail with -ENOMEM after it has already linked
intermediate nodes into the tree, leaving them with no entries.
uverbs_destroy_api() only removes populated slots through
uapi_remove_range(), and radix tree nodes are freed only as a side
effect of deleting an entry, so these empty nodes are leaked when the
uapi is destroyed.

Call xa_destroy() after removing the entries so that all remaining
nodes are freed.

Reported-by: syzbot+d60be66dd00b4490fa96@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d60be66dd00b4490fa96
Fixes: 9ed3e5f44772 ("IB/uverbs: Build the specs into a radix tree at runtime")
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
Changes in v2:
- Drop the dummy insert/delete workaround in uapi_add_elm()
- Call xa_destroy() in uverbs_destroy_api() to free all remaining
  nodes, as suggested by Jason

Compile tested only.
---
 drivers/infiniband/core/uverbs_uapi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
index b67db3790f03..77a4eb494936 100644
--- a/drivers/infiniband/core/uverbs_uapi.c
+++ b/drivers/infiniband/core/uverbs_uapi.c
@@ -621,6 +621,7 @@ void uverbs_destroy_api(struct uverbs_api *uapi)
 		return;
 
 	uapi_remove_range(uapi, 0, U32_MAX);
+	xa_destroy(&uapi->radix);
 	kfree(uapi->write_methods);
 	kfree(uapi);
 }
-- 
2.43.0

Re: [PATCH v2] RDMA/uverbs: Free all radix tree nodes in uverbs_destroy_api()

[syzbot]

> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

Patch testing via #syz test is temporarily disabled.
See https://github.com/google/syzkaller/issues/8122 for details.

>
>
> syzbot reported a memory leak of radix_tree_node objects allocated
> while registering a siw device:
>
>   radix_tree_node_alloc+0x81/0x1a0 lib/radix-tree.c:276
>   radix_tree_insert+0x140/0x390 lib/radix-tree.c:712
>   uapi_add_elm+0x61/0x140 uverbs_uapi.c:41
>   uverbs_alloc_api+0x85/0xc50 uverbs_uapi.c:659
>   siw_newlink+0x36a/0x3f0 siw_main.c:433
>
> radix_tree_insert() can fail with -ENOMEM after it has already linked
> intermediate nodes into the tree, leaving them with no entries.
> uverbs_destroy_api() only removes populated slots through
> uapi_remove_range(), and radix tree nodes are freed only as a side
> effect of deleting an entry, so these empty nodes are leaked when the
> uapi is destroyed.
>
> Call xa_destroy() after removing the entries so that all remaining
> nodes are freed.
>
> Reported-by: syzbot+d60be66dd00b4490fa96@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d60be66dd00b4490fa96
> Fixes: 9ed3e5f44772 ("IB/uverbs: Build the specs into a radix tree at runtime")
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> Changes in v2:
> - Drop the dummy insert/delete workaround in uapi_add_elm()
> - Call xa_destroy() in uverbs_destroy_api() to free all remaining
>   nodes, as suggested by Jason
> ---
>  drivers/infiniband/core/uverbs_uapi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
> index b67db3790f03..77a4eb494936 100644
> --- a/drivers/infiniband/core/uverbs_uapi.c
> +++ b/drivers/infiniband/core/uverbs_uapi.c
> @@ -621,6 +621,7 @@ void uverbs_destroy_api(struct uverbs_api *uapi)
>  		return;
>  
>  	uapi_remove_range(uapi, 0, U32_MAX);
> +	xa_destroy(&uapi->radix);
>  	kfree(uapi->write_methods);
>  	kfree(uapi);
>  }
> -- 
> 2.43.0
>

Re: [PATCH v2] RDMA/uverbs: Free all radix tree nodes in uverbs_destroy_api()

[syzbot]

> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

Patch testing via #syz test is temporarily disabled.
See https://github.com/google/syzkaller/issues/8122 for details.

>
>
> syzbot reported a memory leak of radix_tree_node objects allocated
> while registering a siw device:
>
>   radix_tree_node_alloc+0x81/0x1a0 lib/radix-tree.c:276
>   radix_tree_insert+0x140/0x390 lib/radix-tree.c:712
>   uapi_add_elm+0x61/0x140 uverbs_uapi.c:41
>   uverbs_alloc_api+0x85/0xc50 uverbs_uapi.c:659
>   siw_newlink+0x36a/0x3f0 siw_main.c:433
>
> radix_tree_insert() can fail with -ENOMEM after it has already linked
> intermediate nodes into the tree, leaving them with no entries.
> uverbs_destroy_api() only removes populated slots through
> uapi_remove_range(), and radix tree nodes are freed only as a side
> effect of deleting an entry, so these empty nodes are leaked when the
> uapi is destroyed.
>
> Call xa_destroy() after removing the entries so that all remaining
> nodes are freed.
>
> Reported-by: syzbot+d60be66dd00b4490fa96@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d60be66dd00b4490fa96
> Fixes: 9ed3e5f44772 ("IB/uverbs: Build the specs into a radix tree at runtime")
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> Changes in v2:
> - Drop the dummy insert/delete workaround in uapi_add_elm()
> - Call xa_destroy() in uverbs_destroy_api() to free all remaining
>   nodes, as suggested by Jason
> ---
>  drivers/infiniband/core/uverbs_uapi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/infiniband/core/uverbs_uapi.c b/drivers/infiniband/core/uverbs_uapi.c
> index b67db3790f03..77a4eb494936 100644
> --- a/drivers/infiniband/core/uverbs_uapi.c
> +++ b/drivers/infiniband/core/uverbs_uapi.c
> @@ -621,6 +621,7 @@ void uverbs_destroy_api(struct uverbs_api *uapi)
>  		return;
>  
>  	uapi_remove_range(uapi, 0, U32_MAX);
> +	xa_destroy(&uapi->radix);
>  	kfree(uapi->write_methods);
>  	kfree(uapi);
>  }
> -- 
> 2.43.0
>