Skip to contentSkip to conversation

net: ipv4: current group_info should be put after using.

PATCH In review Wang, Xiaoming 3 participants 3 messages Apr 14, 2014 – May 11, 2014

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Wang, Xiaoming

author
#0 lore raw

PATCH 1/1 net: ipv4: current group_info should be put after using.

Plug a group_info refcount leak in ping_init. group_info is only needed during initialization and the code failed to release the reference on exit. While here move grabbing the reference to a place where it is actually needed.

Signed-off-by Chuansheng Liu <[email protected]>
Signed-off-by Zhang Dongxing <[email protected]>
Signed-off-by xiaoming wang <[email protected]>
---
 net/ipv4/ping.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)
net/ipv4/ping.c +11 -4
index f4b19e5..8210964 100644
@@ -252,26 +252,33 @@ int ping_init_sock(struct sock *sk)
 {
 	struct net *net = sock_net(sk);
 	kgid_t group = current_egid();
-	struct group_info *group_info = get_current_groups();
-	int i, j, count = group_info->ngroups;
+	struct group_info *group_info;
+	int i, j, count;
 	kgid_t low, high;
+	int ret = 0;
 
 	inet_get_ping_group_range_net(net, &low, &high);
 	if (gid_lte(low, group) && gid_lte(group, high))
 		return 0;
 
+	group_info = get_current_groups();
+	count = group_info->ngroups;
 	for (i = 0; i < group_info->nblocks; i++) {
 		int cp_count = min_t(int, NGROUPS_PER_BLOCK, count);
 		for (j = 0; j < cp_count; j++) {
 			kgid_t gid = group_info->blocks[i][j];
 			if (gid_lte(low, gid) && gid_lte(gid, high))
-				return 0;
+				goto out_release_group;
 		}
 
 		count -= cp_count;
 	}
 
-	return -EACCES;
+	ret = -EACCES;
+
+out_release_group:
+	put_group_info(group_info);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(ping_init_sock);
Signature · 1 line
1.7.1
maintainer
#1 lore raw

From: "Wang, Xiaoming" <[email protected]>
Date: Mon, 14 Apr 2014 12:30:45 -0400

Quoted 9 lines

Plug a group_info refcount leak in ping_init. group_info is only needed during initialization and the code failed to release the reference on exit. While here move grabbing the reference to a place where it is actually needed.

Signed-off-by Chuansheng Liu <[email protected]>
Signed-off-by Zhang Dongxing <[email protected]>
Signed-off-by xiaoming wang <[email protected]>

Applied and queued up for -stable, thanks.

#2 lore raw
Quoted 9 lines · David Miller

From: "Wang, Xiaoming" <[email protected]>
Date: Mon, 14 Apr 2014 12:30:45 -0400

Quoted 6 lines

Plug a group_info refcount leak in ping_init. group_info is only needed during initialization and the code failed to release the reference on exit. While here move grabbing the reference to a place where it is actually needed.

Quoted 5 lines
Quoted 3 lines
Signed-off-by Chuansheng Liu <[email protected]>
Signed-off-by Zhang Dongxing <[email protected]>
Signed-off-by xiaoming wang <[email protected]>

Applied and queued up for -stable, thanks.

I see this only made its way to 3.12 kernel, what is holding up inclusion in other kernels? (or maybe this question should be directed at someone else?)

The bug was introduced with:
commit c319b4d76b9e583a5d88d6bf190e079c4e43213d

Author: Vasiliy Kulikov <[email protected]>
Date:   Fri May 13 10:01:00 2011 +0000

net: ipv4: add IPPROTO_ICMP socket kind

starting with 3.0.

Thanks,

Signature · 1 line
Mateusz Guzik