diff mbox

[5/5] net/netfilter/ipvs: Eliminate memory leak

Message ID Pine.LNX.4.64.1008241639300.15172@ask.diku.dk
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Julia Lawall Aug. 24, 2010, 2:39 p.m. UTC
From: Julia Lawall <julia@diku.dk>

__ip_vs_service_get and __ip_vs_svc_fwm_get increment a reference count, so
that reference count should be decremented before leaving the function in an
error case.

A simplified version of the semantic match that finds this problem is:
(http://coccinelle.lip6.fr/)

// <smpl>
@r exists@
local idexpression x;
expression E;
identifier f1;
iterator I;
@@

x = __ip_vs_service_get(...);
<... when != x
     when != true (x == NULL || ...)
     when != if (...) { <+...x...+> }
     when != I (...) { <+...x...+> }
(
 x == NULL
|
 x == E
|
 x->f1
)
...>
* return ...;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 net/netfilter/ipvs/ip_vs_ctl.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Simon Horman Aug. 25, 2010, 1:34 a.m. UTC | #1
On Tue, Aug 24, 2010 at 04:39:49PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> __ip_vs_service_get and __ip_vs_svc_fwm_get increment a reference count, so
> that reference count should be decremented before leaving the function in an
> error case.
> 
> A simplified version of the semantic match that finds this problem is:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @r exists@
> local idexpression x;
> expression E;
> identifier f1;
> iterator I;
> @@
> 
> x = __ip_vs_service_get(...);
> <... when != x
>      when != true (x == NULL || ...)
>      when != if (...) { <+...x...+> }
>      when != I (...) { <+...x...+> }
> (
>  x == NULL
> |
>  x == E
> |
>  x->f1
> )
> ...>
> * return ...;
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>

Thanks Julia, that looks good to me.

Signed-off-by: Simon Horman <horms@verge.net.au>

Patrick, I guess that this should go through nf-next-2.6.

> 
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 0f0c079..f98169b 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2155,7 +2155,7 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
>  	if (cmd != IP_VS_SO_SET_ADD
>  	    && (svc == NULL || svc->protocol != usvc.protocol)) {
>  		ret = -ESRCH;
> -		goto out_unlock;
> +		goto out_drop_service;
>  	}
>  
>  	switch (cmd) {
> @@ -2189,6 +2189,7 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
>  		ret = -EINVAL;
>  	}
>  
> +out_drop_service:
>  	if (svc)
>  		ip_vs_service_put(svc);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 25, 2010, 11:38 p.m. UTC | #2
From: Simon Horman <horms@verge.net.au>
Date: Wed, 25 Aug 2010 10:34:43 +0900

> On Tue, Aug 24, 2010 at 04:39:49PM +0200, Julia Lawall wrote:
>> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> Thanks Julia, that looks good to me.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> Patrick, I guess that this should go through nf-next-2.6.

Since Patrick appears to be offline, I'll apply this directly so
he has less to do when he comes back online :)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Aug. 26, 2010, 12:32 a.m. UTC | #3
On Wed, Aug 25, 2010 at 04:38:12PM -0700, David Miller wrote:
> From: Simon Horman <horms@verge.net.au>
> Date: Wed, 25 Aug 2010 10:34:43 +0900
> 
> > On Tue, Aug 24, 2010 at 04:39:49PM +0200, Julia Lawall wrote:
> >> Signed-off-by: Julia Lawall <julia@diku.dk>
> > 
> > Thanks Julia, that looks good to me.
> > 
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > Patrick, I guess that this should go through nf-next-2.6.
> 
> Since Patrick appears to be offline, I'll apply this directly so
> he has less to do when he comes back online :)

I have a handful of other changes queued up,
do you want to take them too?

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 26, 2010, 3:56 a.m. UTC | #4
From: Simon Horman <horms@verge.net.au>
Date: Thu, 26 Aug 2010 09:32:05 +0900

> I have a handful of other changes queued up,
> do you want to take them too?

Sure, let's do that.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy Sept. 16, 2010, 5:42 a.m. UTC | #5
On 26.08.2010 01:38, David Miller wrote:
> From: Simon Horman <horms@verge.net.au>
> Date: Wed, 25 Aug 2010 10:34:43 +0900
> 
>> On Tue, Aug 24, 2010 at 04:39:49PM +0200, Julia Lawall wrote:
>>> Signed-off-by: Julia Lawall <julia@diku.dk>
>>
>> Thanks Julia, that looks good to me.
>>
>> Signed-off-by: Simon Horman <horms@verge.net.au>
>>
>> Patrick, I guess that this should go through nf-next-2.6.
> 
> Since Patrick appears to be offline, I'll apply this directly so
> he has less to do when he comes back online :)

Thanks Dave. I should be done catching up by tonight and will start
taking care of IPVS patches again.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 0f0c079..f98169b 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2155,7 +2155,7 @@  do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 	if (cmd != IP_VS_SO_SET_ADD
 	    && (svc == NULL || svc->protocol != usvc.protocol)) {
 		ret = -ESRCH;
-		goto out_unlock;
+		goto out_drop_service;
 	}
 
 	switch (cmd) {
@@ -2189,6 +2189,7 @@  do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 		ret = -EINVAL;
 	}
 
+out_drop_service:
 	if (svc)
 		ip_vs_service_put(svc);