Re: [MPTCP] [PATCH mptcp] net: mptcp: don't leak 'slab' and 'slab_name' on exit

Re: [MPTCP] [PATCH mptcp] net: mptcp: don't leak 'slab' and 'slab_name' on exit

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]


On Wed, 22 May 2019, Davide Caratti wrote:

> fix leak of subflows' slab and slab_name in the (rare) case where
> proto_register(mptcp_prot, 1) returns -ENOBUFS.

Thanks for fixing these leaks, Davide. One comment below:

>
> Fixes: e203b1359cf77 ("mptcp: Associate MPTCP context with TCP socket")
> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> ---
> net/mptcp/subflow.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index bc8e1a0658ec..f93c6d4d7172 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -318,6 +318,8 @@ int subflow_init(void)
> void subflow_exit(void)
> {
> 	tcp_unregister_ulp(&subflow_ulp_ops);
> +	kmem_cache_destroy(subflow_request_sock_ops.slab);
> +	kfree(subflow_request_sock_ops.slab_name);

Looking at where slab_name is allocated, kmem_cache_create() can take a 
pointer to an rodata string. If subflow_request_sock_ops.slab_name were 
set using a string literal instead of kasprintf(), we would save a couple 
of unnecessary allocations the new kfree() would not be needed.


Mat


> }
>
> MODULE_LICENSE("GPL");
> -- 
> 2.20.1
>

--
Mat Martineau
Intel

Re: [MPTCP] [PATCH mptcp] net: mptcp: don't leak 'slab' and 'slab_name' on exit

[Davide Caratti]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

hello Mat, thanks for looking at this!

On Wed, 2019-05-22 at 14:57 -0700, Mat Martineau wrote:
> On Wed, 22 May 2019, Davide Caratti wrote:
> 
> Looking at where slab_name is allocated, kmem_cache_create() can take a 
> pointer to an rodata string. If subflow_request_sock_ops.slab_name were 
> set using a string literal instead of kasprintf(), we would save a couple 
> of unnecessary allocations the new kfree() would not be needed.

you are right, I will send a v2 in short time.

regards,
-- 
davide

[MPTCP] [PATCH mptcp] net: mptcp: don't leak 'slab' and 'slab_name' on exit

[Davide Caratti]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

fix leak of subflows' slab and slab_name in the (rare) case where
proto_register(mptcp_prot, 1) returns -ENOBUFS.

Fixes: e203b1359cf77 ("mptcp: Associate MPTCP context with TCP socket")
Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
---
 net/mptcp/subflow.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index bc8e1a0658ec..f93c6d4d7172 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -318,6 +318,8 @@ int subflow_init(void)
 void subflow_exit(void)
 {
 	tcp_unregister_ulp(&subflow_ulp_ops);
+	kmem_cache_destroy(subflow_request_sock_ops.slab);
+	kfree(subflow_request_sock_ops.slab_name);
 }
 
 MODULE_LICENSE("GPL");
-- 
2.20.1