Skip to contentSkip to conversation

netfilter: ipset: off by one in ip_set_nfnl_get_byindex()

PATCH Applied Dan Carpenter 3 participants 3 messages Oct 21, 2014 – Oct 22, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 netfilter: ipset: off by one in ip_set_nfnl_get_byindex()

The ->ip_set_list[] array is initialized in ip_set_net_init() and it has ->ip_set_max elements so this check should be >= instead of > otherwise we are off by one.

---
I am not very familiar with this code, so please review cautiously. This is an old bug which should go to -stable.

net/netfilter/ipset/ip_set_core.c +1 -1
index 912e5a0..86f9d76 100644
@@ -659,7 +659,7 @@ ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index)
 	struct ip_set *set;
 	struct ip_set_net *inst = ip_set_pernet(net);
 
-	if (index > inst->ip_set_max)
+	if (index >= inst->ip_set_max)
 		return IPSET_INVALID_ID;
 
 	nfnl_lock(NFNL_SUBSYS_IPSET);
Acked-by on v1 1/1
#1 lore raw
Quoted 15 lines · Dan Carpenter

The ->ip_set_list[] array is initialized in ip_set_net_init() and it has ->ip_set_max elements so this check should be >= instead of > otherwise we are off by one.

---
I am not very familiar with this code, so please review cautiously. This is an old bug which should go to -stable.

net/netfilter/ipset/ip_set_core.c +0 -0
index 912e5a0..86f9d76 100644

net/netfilter/ipset/ip_set_core.c ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index) · line 665

Quoted 8 lines
net/netfilter/ipset/ip_set_core.c +1 -1
@@ -659,7 +659,7 @@ ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index)
 	struct ip_set *set;
 	struct ip_set_net *inst = ip_set_pernet(net);
 
-	if (index > inst->ip_set_max)
+	if (index >= inst->ip_set_max)
 		return IPSET_INVALID_ID;
 
 	nfnl_lock(NFNL_SUBSYS_IPSET);

Absolutely right and it should go to stable too!

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences

          H-1525 Budapest 114, POB. 49, Hungary
maintainer Applied
#2 lore raw
Quoted 23 lines · Jozsef Kadlecsik
Quoted 21 lines · Dan Carpenter

The ->ip_set_list[] array is initialized in ip_set_net_init() and it has ->ip_set_max elements so this check should be >= instead of > otherwise we are off by one.

---
I am not very familiar with this code, so please review cautiously. This is an old bug which should go to -stable.

net/netfilter/ipset/ip_set_core.c +1 -1
index 912e5a0..86f9d76 100644
@@ -659,7 +659,7 @@ ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index)
 	struct ip_set *set;
 	struct ip_set_net *inst = ip_set_pernet(net);
 
-	if (index > inst->ip_set_max)
+	if (index >= inst->ip_set_max)
 		return IPSET_INVALID_ID;
 
Quoted 6 lines

net/netfilter/ipset/ip_set_core.c ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index) · line 665

Quoted 2 lines
net/netfilter/ipset/ip_set_core.c +0 -0
 	nfnl_lock(NFNL_SUBSYS_IPSET);

Absolutely right and it should go to stable too!

Applied, thanks.

My script says this applies cleanly to:

3.14.x
3.16.x
3.17.x

So I'll enqueue this for those.