* [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][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
* 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
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.