Skip to contentSkip to conversation

SUNRPC: fix memory leak of peer addresses in XPRT

PATCH Superseded Kinglong Mee 2 participants 7 messages Jan 6, 2014 – Mar 29, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

---
 net/sunrpc/xprtsock.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/sunrpc/xprtsock.c +9 -2
index 25dbfa9..11ceba3 100644
@@ -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);
Signature · 1 line
1.8.4.2
#1 lore raw
Quoted 25 lines · Kinglong Mee

If creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

---
 net/sunrpc/xprtsock.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/sunrpc/xprtsock.c +3 -1
index 25dbfa9..11ceba3 100644
@@ -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);

net/sunrpc/xprtsock.c static struct rpc_xprt *xs_setup_local(struct xprt_create *args) · line 2747

Quoted 8 lines
net/sunrpc/xprtsock.c +2 -0
@@ -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?

Quoted 23 lines
net/sunrpc/xprtsock.c +4 -1
@@ -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);
Signature · 1 line
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.

#2 lore raw
Quoted 66 lines · J. Bruce Fields
Quoted 25 lines · Kinglong Mee

If creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

---
 net/sunrpc/xprtsock.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/sunrpc/xprtsock.c +3 -1
index 25dbfa9..11ceba3 100644
@@ -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);

net/sunrpc/xprtsock.c static struct rpc_xprt *xs_setup_local(struct xprt_create *args) · line 2747

Quoted 8 lines
net/sunrpc/xprtsock.c +2 -0
@@ -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?

Quoted 23 lines
net/sunrpc/xprtsock.c +4 -1
@@ -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);
Signature · 1 line
1.8.4.2

And after this we'll end up with

Quoted 5 lines
	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

Quoted 1 line

From 9a53cfcf2c6c9dafeace28ea0c3ac9b3e786412e Mon Sep 17 00:00:00 2001

From: Kinglong Mee <[email protected]>
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.

---
 net/sunrpc/xprtsock.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
net/sunrpc/xprtsock.c +11 -7
index 25dbfa9..4fcdf74 100644
@@ -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;
 }
 
Signature · 1 line
1.8.4.2
#3 lore raw

Dear Bruce,

what's the status of this patch?

thanks,
Kinglong Mee

Quoted 166 lines · Kinglong Mee
Quoted 66 lines · J. Bruce Fields
Quoted 25 lines · Kinglong Mee

If creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

---
 net/sunrpc/xprtsock.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/sunrpc/xprtsock.c +3 -1
index 25dbfa9..11ceba3 100644
@@ -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);

net/sunrpc/xprtsock.c static struct rpc_xprt *xs_setup_local(struct xprt_create *args) · line 2747

Quoted 8 lines
net/sunrpc/xprtsock.c +2 -0
@@ -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?

Quoted 22 lines
net/sunrpc/xprtsock.c +4 -2
@@ -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);
--
Quoted 1 line

1.8.4.2

And after this we'll end up with

Quoted 5 lines
      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 <[email protected]>
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.

---
 net/sunrpc/xprtsock.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
net/sunrpc/xprtsock.c +11 -8
index 25dbfa9..4fcdf74 100644
@@ -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

#4 lore raw

PATCH RESEND 1/1 SUNRPC: fix memory leak of peer addresses in XPRT

Creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

---
 net/sunrpc/xprtsock.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
net/sunrpc/xprtsock.c +11 -8
index 0addefc..2cbafa7 100644
@@ -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;
 }
 
Signature · 1 line
1.8.5.3
#5 lore raw
Quoted 2 lines · Kinglong Mee

Creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

Looks right, thanks--applying.

--b.

Quoted 83 lines
---
 net/sunrpc/xprtsock.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
net/sunrpc/xprtsock.c +11 -8
index 0addefc..2cbafa7 100644
@@ -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;
 }
 
Signature · 1 line
1.8.5.3
#6 lore raw
In reply to Kinglong Mee
Quoted 1 line · Kinglong Mee

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.

Quoted 172 lines

thanks,
Kinglong Mee

Quoted 166 lines · Kinglong Mee
Quoted 66 lines · J. Bruce Fields
Quoted 25 lines · Kinglong Mee

If creating xprt failed after xs_format_peer_addresses, sunrpc must free those memory of peer addresses in xprt.

---
 net/sunrpc/xprtsock.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/sunrpc/xprtsock.c +3 -1
index 25dbfa9..11ceba3 100644
@@ -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);

net/sunrpc/xprtsock.c static struct rpc_xprt *xs_setup_local(struct xprt_create *args) · line 2747

Quoted 8 lines
net/sunrpc/xprtsock.c +2 -0
@@ -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?

Quoted 22 lines
net/sunrpc/xprtsock.c +4 -2
@@ -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);
--
Quoted 1 line

1.8.4.2

And after this we'll end up with

Quoted 5 lines
      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 <[email protected]>
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.

---
 net/sunrpc/xprtsock.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
net/sunrpc/xprtsock.c +11 -8
index 25dbfa9..4fcdf74 100644
@@ -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