Re: [CHECKER] 32 Memory Leaks on Error Paths

Re: [CHECKER] 32 Memory Leaks on Error Paths

[Jörn Engel]

On Mon, 15 September 2003 21:35:46 -0700, David Yu Chen wrote:
> 
> [FILE:  2.6.0-test5/net/ipv4/igmp.c]
> [FUNC:  igmpv3_newpack]
> [LINES: 274-284]
> [VAR:   skb]
>  269:	struct sk_buff *skb;
>  270:	struct rtable *rt;
>  271:	struct iphdr *pip;
>  272:	struct igmpv3_report *pig;
>  273:
> START -->
>  274:	skb = alloc_skb(size + dev->hard_header_len + 15, GFP_ATOMIC);
>  275:	if (skb == NULL)
>  276:		return 0;
>  277:
>  278:	{
>  279:		struct flowi fl = { .oif = dev->ifindex,
>  280:				    .nl_u = { .ip4_u = {
>  281:				    .daddr = IGMPV3_ALL_MCR } },
>  282:				    .proto = IPPROTO_IGMP };
>  283:		if (ip_route_output_key(&rt, &fl))
> END -->
>  284:			return 0;
>  285:	}
>  286:	if (rt->rt_src == 0) {
>  287:		ip_rt_put(rt);
>  288:		return 0;
>  289:	}

Looks valid.  And since skb isn't really needed until after these
returns, moving four lines down a bit fixes the problem.

Davem, is this correct?

Jörn

-- 
Data dominates. If you've chosen the right data structures and organized
things well, the algorithms will almost always be self-evident. Data
structures, not algorithms, are central to programming.
-- Rob Pike

--- linux-2.6.0-test3/net/ipv4/igmp.c~igmp_memleak	2003-07-15 23:09:02.000000000 +0200
+++ linux-2.6.0-test3/net/ipv4/igmp.c	2003-09-16 10:29:47.000000000 +0200
@@ -70,6 +70,8 @@
  *		Alexey Kuznetsov:	Accordance to igmp-v2-06 draft.
  *		David L Stevens:	IGMPv3 support, with help from
  *					Vinay Kulkarni
+ *		Jörn Engel:		Fix memleak in igmpv3_newpack, reported
+ *					by David Yu Chen (stanford checker)
  */
 
 
@@ -271,10 +273,6 @@
 	struct iphdr *pip;
 	struct igmpv3_report *pig;
 
-	skb = alloc_skb(size + dev->hard_header_len + 15, GFP_ATOMIC);
-	if (skb == NULL)
-		return 0;
-
 	{
 		struct flowi fl = { .oif = dev->ifindex,
 				    .nl_u = { .ip4_u = {
@@ -288,6 +286,10 @@
 		return 0;
 	}
 
+	skb = alloc_skb(size + dev->hard_header_len + 15, GFP_ATOMIC);
+	if (skb == NULL)
+		return 0;
+
 	skb->dst = &rt->u.dst;
 	skb->dev = dev;
 

Re: [CHECKER] 32 Memory Leaks on Error Paths

[David S. Miller]

On Tue, 16 Sep 2003 11:07:48 +0200
Jörn Engel <joern@wohnheim.fh-wedel.de> wrote:

> Looks valid.  And since skb isn't really needed until after these
> returns, moving four lines down a bit fixes the problem.
> 
> Davem, is this correct?

Nope, now you're leaking the route instead of the SKB :-)

I'll put the correct fix in.