[PATCH v5] netfilter: ipset: Fix serious failure in CIDR tracking

[PATCH v5] netfilter: ipset: Fix serious failure in CIDR tracking

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This fixes a serious bug affecting all hash types with a net element -
specifically, if a CIDR value is deleted such that none of the same size
exist any more, all larger (less-specific) values will then fail to
match. Adding back any prefix with a CIDR equal to or more specific than
the one deleted will fix it.

Steps to reproduce:
ipset -N test hash:net
ipset -A test 1.1.0.0/16
ipset -A test 2.2.2.0/24
ipset -T test 1.1.1.1		#1.1.1.1 IS in set
ipset -D test 2.2.2.0/24
ipset -T test 1.1.1.1		#1.1.1.1 IS NOT in set

This is due to the fact that the nets counter was unconditionally
decremented prior to the iteration that shifts up the entries. Now, we
first check if there is a proceeding entry and if not, decrement it and
return. Otherwise, we proceed to iterate and then zero the last element,
which, in most cases, will already be zero.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 kernel/net/netfilter/ipset/ip_set_hash_gen.h | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
index 7a5b776..3add0bf 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
@@ -307,18 +307,22 @@ mtype_add_cidr(struct htype *h, u8 cidr, u8 nets_length, u8 n)
 static void
 mtype_del_cidr(struct htype *h, u8 cidr, u8 nets_length, u8 n)
 {
-	u8 i, j;
+	u8 i, j, net_end = nets_length - 1;
 
-	for (i = 0; i < nets_length - 1 && h->nets[i].cidr[n] != cidr; i++)
-		;
-	h->nets[i].nets[n]--;
-
-	if (h->nets[i].nets[n] != 0)
+	for (i = 0; i < nets_length; i++) {
+		if (h->nets[i].cidr[n] != cidr)
+			continue;
+		if (h->nets[i].nets[n] > 1 || i == net_end ||
+		    h->nets[i + 1].nets[n] == 0) {
+			h->nets[i].nets[n]--;
+			return;
+		}
+		for (j = i; j < net_end && h->nets[j].nets[n]; j++) {
+			h->nets[j].cidr[n] = h->nets[j + 1].cidr[n];
+			h->nets[j].nets[n] = h->nets[j + 1].nets[n];
+		}
+		h->nets[j].nets[n] = 0;
 		return;
-
-	for (j = i; j < nets_length - 1 && h->nets[j].nets[n]; j++) {
-		h->nets[j].cidr[n] = h->nets[j + 1].cidr[n];
-		h->nets[j].nets[n] = h->nets[j + 1].nets[n];
 	}
 }
 #endif
-- 
1.8.3.2

Re: [PATCH v5] netfilter: ipset: Fix serious failure in CIDR tracking

[Jozsef Kadlecsik]

On Sat, 14 Sep 2013, Oliver wrote:

> From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> 
> This fixes a serious bug affecting all hash types with a net element -
> specifically, if a CIDR value is deleted such that none of the same size
> exist any more, all larger (less-specific) values will then fail to
> match. Adding back any prefix with a CIDR equal to or more specific than
> the one deleted will fix it.
> 
> Steps to reproduce:
> ipset -N test hash:net
> ipset -A test 1.1.0.0/16
> ipset -A test 2.2.2.0/24
> ipset -T test 1.1.1.1		#1.1.1.1 IS in set
> ipset -D test 2.2.2.0/24
> ipset -T test 1.1.1.1		#1.1.1.1 IS NOT in set
> 
> This is due to the fact that the nets counter was unconditionally
> decremented prior to the iteration that shifts up the entries. Now, we
> first check if there is a proceeding entry and if not, decrement it and
> return. Otherwise, we proceed to iterate and then zero the last element,
> which, in most cases, will already be zero.
> 
> Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

Patch is applied, thanks. I'm going to release a new ipset version next 
week.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
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

Re: [PATCH v5] netfilter: ipset: Fix serious failure in CIDR tracking

[Oliver]

On Sunday 15 September 2013 15:01:36 Jozsef Kadlecsik wrote:
> Patch is applied, thanks. I'm going to release a new ipset version next
> week.
> 
> Best regards,
> Jozsef
> -
> E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
> 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

Excellent, can you possibly make that a friday, I should have a chance to get 
rebased versions of hash:net,net and comments done a few days before then with 
in hope of getting one or t'other committed.

Also, I'd very much like to see this bugfix in the stable kernels, I'm not 
exactly sure what the etiquette is on getting it into the pre-requisite 
mainline fastest? do I submit it to DaveM's net mailing list or straight to 
LKML, or something else entirely?

Thanks,
Oliver.

Re: [PATCH v5] netfilter: ipset: Fix serious failure in CIDR tracking

[Jozsef Kadlecsik]

On Mon, 16 Sep 2013, Oliver wrote:

> On Sunday 15 September 2013 15:01:36 Jozsef Kadlecsik wrote:
> > Patch is applied, thanks. I'm going to release a new ipset version next
> > week.
> 
> Excellent, can you possibly make that a friday, I should have a chance 
> to get rebased versions of hash:net,net and comments done a few days 
> before then with in hope of getting one or t'other committed.

The release can wait till Friday.
 
> Also, I'd very much like to see this bugfix in the stable kernels, I'm not 
> exactly sure what the etiquette is on getting it into the pre-requisite 
> mainline fastest? do I submit it to DaveM's net mailing list or straight to 
> LKML, or something else entirely?

The normal way is that I collect the patches and send them to Pablo who 
then submits those to Dave.

It turned out that the stable kernels lack a lot of bugfixes, so for a 
couple of days I have been working on identifying the missing patches. 
Some backporting is required, so it'll still take some time for me to 
complete and update the trees.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
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