[PATCH] sunrpc: fix leak on error on socket xprt setup

[PATCH] sunrpc: fix leak on error on socket xprt setup

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

Also collect exit code together while we're at it.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 net/sunrpc/xprtsock.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

I think I sent this before, but it must have fallen through the cracks.

--b.

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 02fc7f0..8bfb386 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2297,6 +2297,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
 	struct sockaddr *addr = args->dstaddr;
 	struct rpc_xprt *xprt;
 	struct sock_xprt *transport;
+	struct rpc_xprt *ret;
 
 	xprt = xs_setup_xprt(args, xprt_udp_slot_table_entries);
 	if (IS_ERR(xprt))
@@ -2334,8 +2335,8 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
 		xs_format_peer_addresses(xprt, "udp", RPCBIND_NETID_UDP6);
 		break;
 	default:
-		kfree(xprt);
-		return ERR_PTR(-EAFNOSUPPORT);
+		ret = ERR_PTR(-EAFNOSUPPORT);
+		goto out_err;
 	}
 
 	if (xprt_bound(xprt))
@@ -2350,10 +2351,11 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
 
 	if (try_module_get(THIS_MODULE))
 		return xprt;
-
+	ret = ERR_PTR(-EINVAL);
+out_err:
 	kfree(xprt->slot);
 	kfree(xprt);
-	return ERR_PTR(-EINVAL);
+	return ret;
 }
 
 static const struct rpc_timeout xs_tcp_default_timeout = {
@@ -2372,6 +2374,7 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
 	struct sockaddr *addr = args->dstaddr;
 	struct rpc_xprt *xprt;
 	struct sock_xprt *transport;
+	struct rpc_xprt *ret;
 
 	xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
 	if (IS_ERR(xprt))
@@ -2407,8 +2410,8 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
 		xs_format_peer_addresses(xprt, "tcp", RPCBIND_NETID_TCP6);
 		break;
 	default:
-		kfree(xprt);
-		return ERR_PTR(-EAFNOSUPPORT);
+		ret = ERR_PTR(-EAFNOSUPPORT);
+		goto out_err;
 	}
 
 	if (xprt_bound(xprt))
@@ -2424,10 +2427,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
 
 	if (try_module_get(THIS_MODULE))
 		return xprt;
-
+	ret = ERR_PTR(-EINVAL);
+out_err:
 	kfree(xprt->slot);
 	kfree(xprt);
-	return ERR_PTR(-EINVAL);
+	return ret;
 }
 
 /**
@@ -2441,6 +2445,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
 	struct rpc_xprt *xprt;
 	struct sock_xprt *transport;
 	struct svc_sock *bc_sock;
+	struct rpc_xprt *ret;
 
 	xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
 	if (IS_ERR(xprt))
@@ -2480,8 +2485,8 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
 				   RPCBIND_NETID_TCP6);
 		break;
 	default:
-		kfree(xprt);
-		return ERR_PTR(-EAFNOSUPPORT);
+		ret = ERR_PTR(-EAFNOSUPPORT);
+		goto out_err;
 	}
 
 	if (xprt_bound(xprt))
@@ -2503,9 +2508,11 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
 
 	if (try_module_get(THIS_MODULE))
 		return xprt;
+	ret = ERR_PTR(-EINVAL);
+out_err:
 	kfree(xprt->slot);
 	kfree(xprt);
-	return ERR_PTR(-EINVAL);
+	return ret;
 }
 
 static struct xprt_class	xs_udp_transport = {
-- 
1.7.0.4

Re: [PATCH] sunrpc: fix leak on error on socket xprt setup

[Trond Myklebust]

On Thu, 2010-05-20 at 20:25 -0400, J. Bruce Fields wrote: 
> From: J. Bruce Fields <bfields@citi.umich.edu>
> 
> Also collect exit code together while we're at it.
> 
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  net/sunrpc/xprtsock.c |   29 ++++++++++++++++++-----------
>  1 files changed, 18 insertions(+), 11 deletions(-)
> 
> I think I sent this before, but it must have fallen through the cracks.

The patch doesn't really ring a bell, but it looks correct. On the other
hand, this doesn't look like a bug we'd ever hit in practice, so I'll
probably delay merging it until I've got a few more critical bugfixes
queued up.

Unless, of course, you'd prefer to merge it yourself?

Cheers
  Trond

Re: [PATCH] sunrpc: fix leak on error on socket xprt setup

[J. Bruce Fields]

On Fri, May 21, 2010 at 09:16:19AM -0400, Trond Myklebust wrote:
> On Thu, 2010-05-20 at 20:25 -0400, J. Bruce Fields wrote: 
> > From: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > Also collect exit code together while we're at it.
> > 
> > Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> > Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> >  net/sunrpc/xprtsock.c |   29 ++++++++++++++++++-----------
> >  1 files changed, 18 insertions(+), 11 deletions(-)
> > 
> > I think I sent this before, but it must have fallen through the cracks.
> 
> The patch doesn't really ring a bell, but it looks correct. On the other
> hand, this doesn't look like a bug we'd ever hit in practice,

... and even if we do, the consequences aren't much.

> so I'll
> probably delay merging it until I've got a few more critical bugfixes
> queued up.

I'd've probably left it to the next merge window.

> Unless, of course, you'd prefer to merge it yourself?

No preference; if you're in no hurry I'd just add it to my for-2.6.36
branch with your ack.

--b.