Patchwork XFRM: Fix memory leak in xfrm_state_update

login
register
mail settings
Submitter Tushar Gohad
Date July 8, 2011, 1:38 a.m.
Message ID <alpine.LFD.1.10.1107071820060.7677@net-test1.az.mvista.com>
Download mbox | patch
Permalink /patch/103750/
State Accepted
Delegated to: David Miller
Headers show

Comments

Tushar Gohad - July 8, 2011, 1:38 a.m.
Upon "ip xfrm state update ..", xfrm_add_sa() takes an extra reference on 
the user-supplied SA and forgets to drop the reference when 
xfrm_state_update() returns 0.  This leads to a memory leak as the 
parameter SA is never freed.  This change attempts to fix the leak by 
calling __xfrm_state_put() when xfrm_state_update() updates a valid SA 
(err = 0).  The parameter SA is added to the gc list when the final 
reference is dropped by xfrm_add_sa() upon completion.

Signed-off-by: Tushar Gohad <tgohad@mvista.com>
---
  net/xfrm/xfrm_state.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
Herbert Xu - July 8, 2011, 3 a.m.
On Thu, Jul 07, 2011 at 06:38:52PM -0700, Tushar Gohad wrote:
>
> Upon "ip xfrm state update ..", xfrm_add_sa() takes an extra reference on 
> the user-supplied SA and forgets to drop the reference when  
> xfrm_state_update() returns 0.  This leads to a memory leak as the  
> parameter SA is never freed.  This change attempts to fix the leak by  
> calling __xfrm_state_put() when xfrm_state_update() updates a valid SA  
> (err = 0).  The parameter SA is added to the gc list when the final  
> reference is dropped by xfrm_add_sa() upon completion.
>
> Signed-off-by: Tushar Gohad <tgohad@mvista.com>

Ouch, thanks for catching this bug!

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
David Miller - July 8, 2011, 3:59 p.m.
From: Herbert Xu <herbert@gondor.hengli.com.au>
Date: Fri, 8 Jul 2011 11:00:32 +0800

> On Thu, Jul 07, 2011 at 06:38:52PM -0700, Tushar Gohad wrote:
>>
>> Upon "ip xfrm state update ..", xfrm_add_sa() takes an extra reference on 
>> the user-supplied SA and forgets to drop the reference when  
>> xfrm_state_update() returns 0.  This leads to a memory leak as the  
>> parameter SA is never freed.  This change attempts to fix the leak by  
>> calling __xfrm_state_put() when xfrm_state_update() updates a valid SA  
>> (err = 0).  The parameter SA is added to the gc list when the final  
>> reference is dropped by xfrm_add_sa() upon completion.
>>
>> Signed-off-by: Tushar Gohad <tgohad@mvista.com>
> 
> Ouch, thanks for catching this bug!
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, 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

Patch

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index d70f85e..9414b9c 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1345,6 +1345,8 @@  out:
  			xfrm_state_check_expire(x1);

  		err = 0;
+		x->km.state = XFRM_STATE_DEAD;
+		__xfrm_state_put(x);
  	}
  	spin_unlock_bh(&x1->lock);