* [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
@ 2014-01-06 10:24 Kinglong Mee
2014-01-06 20:20 ` J. Bruce Fields
0 siblings, 1 reply; 7+ messages in thread
From: Kinglong Mee @ 2014-01-06 10:24 UTC (permalink / raw)
To: J. Bruce Fields, Trond Myklebust, Linux NFS Mailing List
If creating xprt failed after xs_format_peer_addresses,
sunrpc must free those memory of peer addresses in xprt.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
net/sunrpc/xprtsock.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 25dbfa9..11ceba3 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2725,8 +2725,10 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
xprt_set_bound(xprt);
xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
ret = ERR_PTR(xs_local_setup_socket(transport));
- if (ret)
+ if (ret) {
+ xs_free_peer_addresses(xprt);
goto out_err;
+ }
break;
default:
ret = ERR_PTR(-EAFNOSUPPORT);
@@ -2738,6 +2740,8 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
if (try_module_get(THIS_MODULE))
return xprt;
+
+ xs_free_peer_addresses(xprt);
ret = ERR_PTR(-EINVAL);
out_err:
xprt_free(xprt);
@@ -2816,6 +2820,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
if (try_module_get(THIS_MODULE))
return xprt;
+
+ xs_free_peer_addresses(xprt);
ret = ERR_PTR(-EINVAL);
out_err:
xprt_free(xprt);
@@ -2893,9 +2899,10 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
xprt->address_strings[RPC_DISPLAY_ADDR],
xprt->address_strings[RPC_DISPLAY_PROTO]);
-
if (try_module_get(THIS_MODULE))
return xprt;
+
+ xs_free_peer_addresses(xprt);
ret = ERR_PTR(-EINVAL);
out_err:
xprt_free(xprt);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
2014-01-06 10:24 [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT Kinglong Mee
@ 2014-01-06 20:20 ` J. Bruce Fields
2014-01-07 3:49 ` Kinglong Mee
0 siblings, 1 reply; 7+ messages in thread
From: J. Bruce Fields @ 2014-01-06 20:20 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Trond Myklebust, Linux NFS Mailing List
On Mon, Jan 06, 2014 at 06:24:32PM +0800, Kinglong Mee wrote:
> If creating xprt failed after xs_format_peer_addresses,
> sunrpc must free those memory of peer addresses in xprt.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> net/sunrpc/xprtsock.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 25dbfa9..11ceba3 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2725,8 +2725,10 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> xprt_set_bound(xprt);
> xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
> ret = ERR_PTR(xs_local_setup_socket(transport));
> - if (ret)
> + if (ret) {
> + xs_free_peer_addresses(xprt);
> goto out_err;
> + }
> break;
> default:
> ret = ERR_PTR(-EAFNOSUPPORT);
> @@ -2738,6 +2740,8 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
>
> if (try_module_get(THIS_MODULE))
> return xprt;
> +
> + xs_free_peer_addresses(xprt);
> ret = ERR_PTR(-EINVAL);
> out_err:
> xprt_free(xprt);
This is getting a little hairy.... Looks like xprts are alloc'd with
kzalloc() and xs_free_peer_addresses is a no-op if
xprt->address_strings[i] is NULL, so it looks safe to call
unconditionally after out_err?
> @@ -2816,6 +2820,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
>
> if (try_module_get(THIS_MODULE))
> return xprt;
> +
> + xs_free_peer_addresses(xprt);
> ret = ERR_PTR(-EINVAL);
> out_err:
> xprt_free(xprt);
> @@ -2893,9 +2899,10 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> xprt->address_strings[RPC_DISPLAY_ADDR],
> xprt->address_strings[RPC_DISPLAY_PROTO]);
>
> -
> if (try_module_get(THIS_MODULE))
> return xprt;
> +
> + xs_free_peer_addresses(xprt);
> ret = ERR_PTR(-EINVAL);
> out_err:
> xprt_free(xprt);
> --
> 1.8.4.2
And after this we'll end up with
xs_free_peer_addresses(xprt);
xprt_free(xprt);
in 4 different places (the above plus xs_destroy), so I might define an
xs_xprt_free() to do that.
--b.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
2014-01-06 20:20 ` J. Bruce Fields
@ 2014-01-07 3:49 ` Kinglong Mee
2014-03-23 13:24 ` Kinglong Mee
0 siblings, 1 reply; 7+ messages in thread
From: Kinglong Mee @ 2014-01-07 3:49 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Trond Myklebust, Linux NFS Mailing List
On 01/07/2014 04:20 AM, J. Bruce Fields wrote:
> On Mon, Jan 06, 2014 at 06:24:32PM +0800, Kinglong Mee wrote:
>> If creating xprt failed after xs_format_peer_addresses,
>> sunrpc must free those memory of peer addresses in xprt.
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>> net/sunrpc/xprtsock.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>> index 25dbfa9..11ceba3 100644
>> --- a/net/sunrpc/xprtsock.c
>> +++ b/net/sunrpc/xprtsock.c
>> @@ -2725,8 +2725,10 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
>> xprt_set_bound(xprt);
>> xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
>> ret = ERR_PTR(xs_local_setup_socket(transport));
>> - if (ret)
>> + if (ret) {
>> + xs_free_peer_addresses(xprt);
>> goto out_err;
>> + }
>> break;
>> default:
>> ret = ERR_PTR(-EAFNOSUPPORT);
>> @@ -2738,6 +2740,8 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
>>
>> if (try_module_get(THIS_MODULE))
>> return xprt;
>> +
>> + xs_free_peer_addresses(xprt);
>> ret = ERR_PTR(-EINVAL);
>> out_err:
>> xprt_free(xprt);
>
> This is getting a little hairy.... Looks like xprts are alloc'd with
> kzalloc() and xs_free_peer_addresses is a no-op if
> xprt->address_strings[i] is NULL, so it looks safe to call
> unconditionally after out_err?
>
>> @@ -2816,6 +2820,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
>>
>> if (try_module_get(THIS_MODULE))
>> return xprt;
>> +
>> + xs_free_peer_addresses(xprt);
>> ret = ERR_PTR(-EINVAL);
>> out_err:
>> xprt_free(xprt);
>> @@ -2893,9 +2899,10 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
>> xprt->address_strings[RPC_DISPLAY_ADDR],
>> xprt->address_strings[RPC_DISPLAY_PROTO]);
>>
>> -
>> if (try_module_get(THIS_MODULE))
>> return xprt;
>> +
>> + xs_free_peer_addresses(xprt);
>> ret = ERR_PTR(-EINVAL);
>> out_err:
>> xprt_free(xprt);
>> --
>> 1.8.4.2
>
> And after this we'll end up with
>
> xs_free_peer_addresses(xprt);
> xprt_free(xprt);
>
> in 4 different places (the above plus xs_destroy), so I might define an
> xs_xprt_free() to do that.
I have make a new patch as your suggestion.
thanks,
Kinglong Mee
>From 9a53cfcf2c6c9dafeace28ea0c3ac9b3e786412e Mon Sep 17 00:00:00 2001
From: Kinglong Mee <kinglongmee@gmail.com>
Date: Tue, 7 Jan 2014 11:43:59 +0800
Subject: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
If creating xprt failed after xs_format_peer_addresses,
sunrpc must free those memory of peer addresses in xprt.
Add a helper function for freeing xprt named xs_xprt_free.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
net/sunrpc/xprtsock.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 25dbfa9..4fcdf74 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -905,6 +905,12 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
xs_tcp_shutdown(xprt);
}
+static void xs_xprt_free(struct rpc_xprt *xprt)
+{
+ xs_free_peer_addresses(xprt);
+ xprt_free(xprt);
+}
+
/**
* xs_destroy - prepare to shutdown a transport
* @xprt: doomed transport
@@ -915,8 +921,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
dprintk("RPC: xs_destroy xprt %p\n", xprt);
xs_close(xprt);
- xs_free_peer_addresses(xprt);
- xprt_free(xprt);
+ xs_xprt_free(xprt);
module_put(THIS_MODULE);
}
@@ -2740,7 +2745,7 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2818,7 +2823,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2893,12 +2898,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
xprt->address_strings[RPC_DISPLAY_ADDR],
xprt->address_strings[RPC_DISPLAY_PROTO]);
-
if (try_module_get(THIS_MODULE))
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2988,7 +2992,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
xprt_put(xprt);
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
2014-01-07 3:49 ` Kinglong Mee
@ 2014-03-23 13:24 ` Kinglong Mee
2014-03-24 3:07 ` [PATCH][RESEND] " Kinglong Mee
2014-03-25 0:05 ` [PATCH] " J. Bruce Fields
0 siblings, 2 replies; 7+ messages in thread
From: Kinglong Mee @ 2014-03-23 13:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Trond Myklebust, Linux NFS Mailing List
Dear Bruce,
what's the status of this patch?
thanks,
Kinglong Mee
On Tue, Jan 7, 2014 at 11:49 AM, Kinglong Mee <kinglongmee@gmail.com> wrote:
> On 01/07/2014 04:20 AM, J. Bruce Fields wrote:
>> On Mon, Jan 06, 2014 at 06:24:32PM +0800, Kinglong Mee wrote:
>>> If creating xprt failed after xs_format_peer_addresses,
>>> sunrpc must free those memory of peer addresses in xprt.
>>>
>>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>>> ---
>>> net/sunrpc/xprtsock.c | 11 +++++++++--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>>> index 25dbfa9..11ceba3 100644
>>> --- a/net/sunrpc/xprtsock.c
>>> +++ b/net/sunrpc/xprtsock.c
>>> @@ -2725,8 +2725,10 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
>>> xprt_set_bound(xprt);
>>> xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
>>> ret = ERR_PTR(xs_local_setup_socket(transport));
>>> - if (ret)
>>> + if (ret) {
>>> + xs_free_peer_addresses(xprt);
>>> goto out_err;
>>> + }
>>> break;
>>> default:
>>> ret = ERR_PTR(-EAFNOSUPPORT);
>>> @@ -2738,6 +2740,8 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
>>>
>>> if (try_module_get(THIS_MODULE))
>>> return xprt;
>>> +
>>> + xs_free_peer_addresses(xprt);
>>> ret = ERR_PTR(-EINVAL);
>>> out_err:
>>> xprt_free(xprt);
>>
>> This is getting a little hairy.... Looks like xprts are alloc'd with
>> kzalloc() and xs_free_peer_addresses is a no-op if
>> xprt->address_strings[i] is NULL, so it looks safe to call
>> unconditionally after out_err?
>>
>>> @@ -2816,6 +2820,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
>>>
>>> if (try_module_get(THIS_MODULE))
>>> return xprt;
>>> +
>>> + xs_free_peer_addresses(xprt);
>>> ret = ERR_PTR(-EINVAL);
>>> out_err:
>>> xprt_free(xprt);
>>> @@ -2893,9 +2899,10 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
>>> xprt->address_strings[RPC_DISPLAY_ADDR],
>>> xprt->address_strings[RPC_DISPLAY_PROTO]);
>>>
>>> -
>>> if (try_module_get(THIS_MODULE))
>>> return xprt;
>>> +
>>> + xs_free_peer_addresses(xprt);
>>> ret = ERR_PTR(-EINVAL);
>>> out_err:
>>> xprt_free(xprt);
>>> --
>>> 1.8.4.2
>>
>> And after this we'll end up with
>>
>> xs_free_peer_addresses(xprt);
>> xprt_free(xprt);
>>
>> in 4 different places (the above plus xs_destroy), so I might define an
>> xs_xprt_free() to do that.
>
> I have make a new patch as your suggestion.
>
> thanks,
> Kinglong Mee
>
> From 9a53cfcf2c6c9dafeace28ea0c3ac9b3e786412e Mon Sep 17 00:00:00 2001
> From: Kinglong Mee <kinglongmee@gmail.com>
> Date: Tue, 7 Jan 2014 11:43:59 +0800
> Subject: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
>
> If creating xprt failed after xs_format_peer_addresses,
> sunrpc must free those memory of peer addresses in xprt.
>
> Add a helper function for freeing xprt named xs_xprt_free.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> net/sunrpc/xprtsock.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 25dbfa9..4fcdf74 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -905,6 +905,12 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
> xs_tcp_shutdown(xprt);
> }
>
> +static void xs_xprt_free(struct rpc_xprt *xprt)
> +{
> + xs_free_peer_addresses(xprt);
> + xprt_free(xprt);
> +}
> +
> /**
> * xs_destroy - prepare to shutdown a transport
> * @xprt: doomed transport
> @@ -915,8 +921,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
> dprintk("RPC: xs_destroy xprt %p\n", xprt);
>
> xs_close(xprt);
> - xs_free_peer_addresses(xprt);
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> module_put(THIS_MODULE);
> }
>
> @@ -2740,7 +2745,7 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2818,7 +2823,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2893,12 +2898,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> xprt->address_strings[RPC_DISPLAY_ADDR],
> xprt->address_strings[RPC_DISPLAY_PROTO]);
>
> -
> if (try_module_get(THIS_MODULE))
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2988,7 +2992,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
> xprt_put(xprt);
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> --
> 1.8.4.2
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH][RESEND] SUNRPC: fix memory leak of peer addresses in XPRT
2014-03-23 13:24 ` Kinglong Mee
@ 2014-03-24 3:07 ` Kinglong Mee
2014-03-29 0:55 ` J. Bruce Fields
2014-03-25 0:05 ` [PATCH] " J. Bruce Fields
1 sibling, 1 reply; 7+ messages in thread
From: Kinglong Mee @ 2014-03-24 3:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Trond Myklebust, Linux NFS Mailing List
Creating xprt failed after xs_format_peer_addresses,
sunrpc must free those memory of peer addresses in xprt.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
net/sunrpc/xprtsock.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 0addefc..2cbafa7 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -909,6 +909,12 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
xs_tcp_shutdown(xprt);
}
+static void xs_xprt_free(struct rpc_xprt *xprt)
+{
+ xs_free_peer_addresses(xprt);
+ xprt_free(xprt);
+}
+
/**
* xs_destroy - prepare to shutdown a transport
* @xprt: doomed transport
@@ -919,8 +925,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
dprintk("RPC: xs_destroy xprt %p\n", xprt);
xs_close(xprt);
- xs_free_peer_addresses(xprt);
- xprt_free(xprt);
+ xs_xprt_free(xprt);
module_put(THIS_MODULE);
}
@@ -2744,7 +2749,7 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2822,7 +2827,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2897,12 +2902,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
xprt->address_strings[RPC_DISPLAY_ADDR],
xprt->address_strings[RPC_DISPLAY_PROTO]);
-
if (try_module_get(THIS_MODULE))
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
@@ -2985,13 +2989,12 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
*/
xprt_set_connected(xprt);
-
if (try_module_get(THIS_MODULE))
return xprt;
xprt_put(xprt);
ret = ERR_PTR(-EINVAL);
out_err:
- xprt_free(xprt);
+ xs_xprt_free(xprt);
return ret;
}
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
2014-03-23 13:24 ` Kinglong Mee
2014-03-24 3:07 ` [PATCH][RESEND] " Kinglong Mee
@ 2014-03-25 0:05 ` J. Bruce Fields
1 sibling, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2014-03-25 0:05 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Trond Myklebust, Linux NFS Mailing List
On Sun, Mar 23, 2014 at 09:24:31PM +0800, Kinglong Mee wrote:
> what's the status of this patch?
I haven't looked at it yet. I'll try to get this and your other patches
reviewed and queued up later this week after getting home from LSF/MM.
Apologies for the confusion, I put off too long getting together my tree
for this merge window....
--b.
>
> thanks,
> Kinglong Mee
>
>
> On Tue, Jan 7, 2014 at 11:49 AM, Kinglong Mee <kinglongmee@gmail.com> wrote:
> > On 01/07/2014 04:20 AM, J. Bruce Fields wrote:
> >> On Mon, Jan 06, 2014 at 06:24:32PM +0800, Kinglong Mee wrote:
> >>> If creating xprt failed after xs_format_peer_addresses,
> >>> sunrpc must free those memory of peer addresses in xprt.
> >>>
> >>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> >>> ---
> >>> net/sunrpc/xprtsock.c | 11 +++++++++--
> >>> 1 file changed, 9 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> >>> index 25dbfa9..11ceba3 100644
> >>> --- a/net/sunrpc/xprtsock.c
> >>> +++ b/net/sunrpc/xprtsock.c
> >>> @@ -2725,8 +2725,10 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> >>> xprt_set_bound(xprt);
> >>> xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
> >>> ret = ERR_PTR(xs_local_setup_socket(transport));
> >>> - if (ret)
> >>> + if (ret) {
> >>> + xs_free_peer_addresses(xprt);
> >>> goto out_err;
> >>> + }
> >>> break;
> >>> default:
> >>> ret = ERR_PTR(-EAFNOSUPPORT);
> >>> @@ -2738,6 +2740,8 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> >>>
> >>> if (try_module_get(THIS_MODULE))
> >>> return xprt;
> >>> +
> >>> + xs_free_peer_addresses(xprt);
> >>> ret = ERR_PTR(-EINVAL);
> >>> out_err:
> >>> xprt_free(xprt);
> >>
> >> This is getting a little hairy.... Looks like xprts are alloc'd with
> >> kzalloc() and xs_free_peer_addresses is a no-op if
> >> xprt->address_strings[i] is NULL, so it looks safe to call
> >> unconditionally after out_err?
> >>
> >>> @@ -2816,6 +2820,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
> >>>
> >>> if (try_module_get(THIS_MODULE))
> >>> return xprt;
> >>> +
> >>> + xs_free_peer_addresses(xprt);
> >>> ret = ERR_PTR(-EINVAL);
> >>> out_err:
> >>> xprt_free(xprt);
> >>> @@ -2893,9 +2899,10 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> >>> xprt->address_strings[RPC_DISPLAY_ADDR],
> >>> xprt->address_strings[RPC_DISPLAY_PROTO]);
> >>>
> >>> -
> >>> if (try_module_get(THIS_MODULE))
> >>> return xprt;
> >>> +
> >>> + xs_free_peer_addresses(xprt);
> >>> ret = ERR_PTR(-EINVAL);
> >>> out_err:
> >>> xprt_free(xprt);
> >>> --
> >>> 1.8.4.2
> >>
> >> And after this we'll end up with
> >>
> >> xs_free_peer_addresses(xprt);
> >> xprt_free(xprt);
> >>
> >> in 4 different places (the above plus xs_destroy), so I might define an
> >> xs_xprt_free() to do that.
> >
> > I have make a new patch as your suggestion.
> >
> > thanks,
> > Kinglong Mee
> >
> > From 9a53cfcf2c6c9dafeace28ea0c3ac9b3e786412e Mon Sep 17 00:00:00 2001
> > From: Kinglong Mee <kinglongmee@gmail.com>
> > Date: Tue, 7 Jan 2014 11:43:59 +0800
> > Subject: [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT
> >
> > If creating xprt failed after xs_format_peer_addresses,
> > sunrpc must free those memory of peer addresses in xprt.
> >
> > Add a helper function for freeing xprt named xs_xprt_free.
> >
> > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> > ---
> > net/sunrpc/xprtsock.c | 18 +++++++++++-------
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 25dbfa9..4fcdf74 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -905,6 +905,12 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
> > xs_tcp_shutdown(xprt);
> > }
> >
> > +static void xs_xprt_free(struct rpc_xprt *xprt)
> > +{
> > + xs_free_peer_addresses(xprt);
> > + xprt_free(xprt);
> > +}
> > +
> > /**
> > * xs_destroy - prepare to shutdown a transport
> > * @xprt: doomed transport
> > @@ -915,8 +921,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
> > dprintk("RPC: xs_destroy xprt %p\n", xprt);
> >
> > xs_close(xprt);
> > - xs_free_peer_addresses(xprt);
> > - xprt_free(xprt);
> > + xs_xprt_free(xprt);
> > module_put(THIS_MODULE);
> > }
> >
> > @@ -2740,7 +2745,7 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> > return xprt;
> > ret = ERR_PTR(-EINVAL);
> > out_err:
> > - xprt_free(xprt);
> > + xs_xprt_free(xprt);
> > return ret;
> > }
> >
> > @@ -2818,7 +2823,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
> > return xprt;
> > ret = ERR_PTR(-EINVAL);
> > out_err:
> > - xprt_free(xprt);
> > + xs_xprt_free(xprt);
> > return ret;
> > }
> >
> > @@ -2893,12 +2898,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> > xprt->address_strings[RPC_DISPLAY_ADDR],
> > xprt->address_strings[RPC_DISPLAY_PROTO]);
> >
> > -
> > if (try_module_get(THIS_MODULE))
> > return xprt;
> > ret = ERR_PTR(-EINVAL);
> > out_err:
> > - xprt_free(xprt);
> > + xs_xprt_free(xprt);
> > return ret;
> > }
> >
> > @@ -2988,7 +2992,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
> > xprt_put(xprt);
> > ret = ERR_PTR(-EINVAL);
> > out_err:
> > - xprt_free(xprt);
> > + xs_xprt_free(xprt);
> > return ret;
> > }
> >
> > --
> > 1.8.4.2
> >
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][RESEND] SUNRPC: fix memory leak of peer addresses in XPRT
2014-03-24 3:07 ` [PATCH][RESEND] " Kinglong Mee
@ 2014-03-29 0:55 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2014-03-29 0:55 UTC (permalink / raw)
To: Kinglong Mee; +Cc: Trond Myklebust, Linux NFS Mailing List
On Mon, Mar 24, 2014 at 11:07:22AM +0800, Kinglong Mee wrote:
> Creating xprt failed after xs_format_peer_addresses,
> sunrpc must free those memory of peer addresses in xprt.
Looks right, thanks--applying.
--b.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> net/sunrpc/xprtsock.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 0addefc..2cbafa7 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -909,6 +909,12 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
> xs_tcp_shutdown(xprt);
> }
>
> +static void xs_xprt_free(struct rpc_xprt *xprt)
> +{
> + xs_free_peer_addresses(xprt);
> + xprt_free(xprt);
> +}
> +
> /**
> * xs_destroy - prepare to shutdown a transport
> * @xprt: doomed transport
> @@ -919,8 +925,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
> dprintk("RPC: xs_destroy xprt %p\n", xprt);
>
> xs_close(xprt);
> - xs_free_peer_addresses(xprt);
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> module_put(THIS_MODULE);
> }
>
> @@ -2744,7 +2749,7 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2822,7 +2827,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2897,12 +2902,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> xprt->address_strings[RPC_DISPLAY_ADDR],
> xprt->address_strings[RPC_DISPLAY_PROTO]);
>
> -
> if (try_module_get(THIS_MODULE))
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> @@ -2985,13 +2989,12 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
> */
> xprt_set_connected(xprt);
>
> -
> if (try_module_get(THIS_MODULE))
> return xprt;
> xprt_put(xprt);
> ret = ERR_PTR(-EINVAL);
> out_err:
> - xprt_free(xprt);
> + xs_xprt_free(xprt);
> return ret;
> }
>
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-29 0:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-06 10:24 [PATCH] SUNRPC: fix memory leak of peer addresses in XPRT Kinglong Mee
2014-01-06 20:20 ` J. Bruce Fields
2014-01-07 3:49 ` Kinglong Mee
2014-03-23 13:24 ` Kinglong Mee
2014-03-24 3:07 ` [PATCH][RESEND] " Kinglong Mee
2014-03-29 0:55 ` J. Bruce Fields
2014-03-25 0:05 ` [PATCH] " J. Bruce Fields
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.