net/ipv4: Eliminate kstrdup memory leak

Julia Lawall · davem330 · commit c34186ed0082 · 2010-08-27T19:31:56.000-07:00
The string clone is only used as a temporary copy of the argument val within the while loop, and so it should be freed before leaving the function. The call to strsep, however, modifies clone, so a pointer to the front of the string is kept in saved_clone, to make it possible to free it. The sematic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> @r exists@ local idexpression x; expression E; identifier l; statement S; @@ *x= \(kasprintf\|kstrdup\)(...); ... if (x == NULL) S ... when != kfree(x) when != E = x if (...) { <... when != kfree(x) * goto l; ...> * return ...; } // </smpl> Signed-off-by: Julia Lawall <julia@diku.dk> Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
@@ -196,10 +196,10 @@ void tcp_get_allowed_congestion_control(char *buf, size_t maxlen)
 int tcp_set_allowed_congestion_control(char *val)
 {
 	struct tcp_congestion_ops *ca;
-	char *clone, *name;
+	char *saved_clone, *clone, *name;
 	int ret = 0;
 
-	clone = kstrdup(val, GFP_USER);
+	saved_clone = clone = kstrdup(val, GFP_USER);
 	if (!clone)
 		return -ENOMEM;
 
@@ -226,6 +226,7 @@ int tcp_set_allowed_congestion_control(char *val)
 	}
 out:
 	spin_unlock(&tcp_cong_list_lock);
+	kfree(saved_clone);
 
 	return ret;
 }

Original file line number	Diff line number	Diff line change
`@@ -196,10 +196,10 @@ void tcp_get_allowed_congestion_control(char *buf, size_t maxlen)`
`196`	`196`	`int tcp_set_allowed_congestion_control(char *val)`
`197`	`197`	`{`
`198`	`198`	`struct tcp_congestion_ops *ca;`
`199`		`- char clone, name;`
	`199`	`+ char saved_clone, clone, *name;`
`200`	`200`	`int ret = 0;`
`201`	`201`
`202`		`- clone = kstrdup(val, GFP_USER);`
	`202`	`+ saved_clone = clone = kstrdup(val, GFP_USER);`
`203`	`203`	`if (!clone)`
`204`	`204`	`return -ENOMEM;`
`205`	`205`
`@@ -226,6 +226,7 @@ int tcp_set_allowed_congestion_control(char *val)`
`226`	`226`	`}`
`227`	`227`	`out:`
`228`	`228`	`spin_unlock(&tcp_cong_list_lock);`
	`229`	`+ kfree(saved_clone);`
`229`	`230`
`230`	`231`	`return ret;`
`231`	`232`	`}`