All of lore.kernel.org
 help / color / mirror / Atom feed
* ipt_recent fix
@ 2006-02-17 17:37 Amin Azez
  2006-02-20 16:12 ` [patch] ipt_recent Amin Azez
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Amin Azez @ 2006-02-17 17:37 UTC (permalink / raw)
  To: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

Here is a fix for ipt_recent.

The problem is that curr_table[x].hash_entry=0 (for all x)

So all pristine (never used) curr_table entries claim to have hash pos 0.

So when we add new items to the hash and do:
  hash_table[r_list[location].hash_entry] = -1;
to use the oldest (used or unused) location - if it is unused we trash 
anything that really has hash pos 0 already allocated.

This can be reproduced with this:

# iptables -A FORWARD -d 1.1.1.1 -m recent --name TEST1 --set
# G=1 ; while test $G -lt 17 ; do echo $G ; echo 1.1.1.$G> \
  /proc/net/ipt_recent/TEST1 ; G=$(($G+1)) ; dmesg -c ; cat \
  /proc/net/ipt_recent/TEST1  | head ; done > x

the 13th item has hash of 0 and gets trashed when the 14th item is added 
without this patch.

We should only trash it then if we _know_ it has been used before and 
therefore is REALLY at the position it claims...

This patch makes sure that items claiming to have a hash entry really do 
have that hash entry before destroying that hash entry.

This patch also adds counting of the number of hash entries in 
preparation for conditions based on the number of entries in the recent 
table rule; patch for that to follow next week.

Any comments on this?

Sam

[-- Attachment #2: ipt_recent.patch --]
[-- Type: text/x-patch, Size: 3157 bytes --]

--- ./net/ipv4/netfilter/ipt_recent.c.nolimit	2006-02-15 16:34:20.000000000 +0000
+++ ./net/ipv4/netfilter/ipt_recent.c	2006-02-17 17:23:00.000000000 +0000
@@ -70,7 +70,11 @@
 /* Structure of our linked list of tables of recent lists. */
 struct recent_ip_tables {
 	char name[IPT_RECENT_NAME_LEN];
+	/* number of entries in list *table */
+	int entry_count;
+	/* number of reference to this structure from iptables rules */
 	int count;
+	/* an index increased with each operation which maps time_info[x].position to a position in table */
 	int time_pos;
 	struct recent_ip_list *table;
 	struct recent_ip_tables *next;
@@ -139,6 +143,7 @@
 	curr_table = (struct recent_ip_tables*) data;
 
 	spin_lock_bh(&curr_table->list_lock);
+	len += sprintf(buffer+len,"count=%d\n",curr_table->entry_count);
 	for(count = 0; count < ip_list_tot; count++) {
 		if(!curr_table->table[count].addr) continue;
 		last_len = len;
@@ -216,6 +221,7 @@
 		used += 5;
 		spin_lock_bh(&curr_table->list_lock);
 		curr_table->time_pos = 0;
+		curr_table->entry_count = 0;
 		for(count = 0; count < ip_list_hash_size; count++) {
 			curr_table->hash_table[count] = -1;
 		}
@@ -500,7 +506,27 @@
 
 		/* New item found and IPT_RECENT_SET, so we need to add it */
 		location = time_info[curr_table->time_pos].position;
-		hash_table[r_list[location].hash_entry] = -1;
+		/* 
+		   location is the next slot in r_list that we will use. 
+		   Maybe it is already used and we need to remove it fromthe hash.
+		   If it is used then: 
+		   1) hash_table[r_list[location].hash_entry]=location
+		   Which is to say that anything claiming to have a hash entry
+		   should also be pointed to BY that hash entry.
+		     This only comes up becase by default r_list[x].hash_entry=0
+		   i.e. all items claim to have hash entry 0, so if a real
+		   item has hash slot 0 it gets trashed because of lies.
+		   SO: If r_list[location].hash_entry]==0 we are in warning condition
+		   and need to double check that hash_table reciprocates before we
+		   set the hash_table entry to -1 (it may not really belong)
+		   This would work; 
+		     if (r_list[location].hash_entry!=0 || hash_table[0]==location) hash_table[r_list[location].hash_entry] = -1; 
+		   but we want to combine with a counter for new entries
+		*/
+		/* increase counter if this is a new item */
+		if (hash_table[r_list[location].hash_entry]!=location) curr_table->entry_count++;
+		else hash_table[r_list[location].hash_entry] = -1;
+
 		hash_table[hash_result] = location;
 		memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 		r_list[location].time_pos = curr_table->time_pos;
@@ -633,6 +659,7 @@
 			r_list[location].ttl = 0;
 			memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 			r_list[location].oldest_pkt = 0;
+			curr_table->entry_count--;
 			ans = !info->invert;
 		}
 		spin_unlock_bh(&curr_table->list_lock);
@@ -719,6 +746,7 @@
 	curr_table->next = NULL;
 	curr_table->count = 1;
 	curr_table->time_pos = 0;
+	curr_table->entry_count = 0;
 	strncpy(curr_table->name,info->name,IPT_RECENT_NAME_LEN);
 	curr_table->name[IPT_RECENT_NAME_LEN-1] = '\0';
 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [patch] ipt_recent
  2006-02-17 17:37 ipt_recent fix Amin Azez
@ 2006-02-20 16:12 ` Amin Azez
  2006-03-04 10:00   ` Patrick McHardy
  2006-02-21 11:16 ` BUG: More ipt_recent queries Amin Azez
  2006-03-04 10:10 ` ipt_recent fix Patrick McHardy
  2 siblings, 1 reply; 16+ messages in thread
From: Amin Azez @ 2006-02-20 16:12 UTC (permalink / raw)
  To: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

This patch fixes the previously mentioned bug in ipt_recent and adds:

--lt n  # check less than n items in list
--gt n  # checks more than n items in list
--eq n  # check exactly n items in list

Which can be prefixed with ! to invert.

These preconditions are checked before any other conditions in 
ipt_recent, and their failure will prevent any other actions or checks 
from being considered.

Thus it is possible to make decisions based on the size of the 
ipt_recent list (number of IP addresses in it).

iptables.recent.patch is the userland patch
ipt_recent.patch is a patch on kernel 2.6.11.7

Sam


[-- Attachment #2: iptables.recent.patch --]
[-- Type: text/x-patch, Size: 4821 bytes --]

--- extensions/libipt_recent.man.nolimit	2006-02-20 13:55:58.000000000 +0000
+++ extensions/libipt_recent.man	2006-02-20 14:10:25.000000000 +0000
@@ -9,6 +9,21 @@
 Specify the list to use for the commands. If no name is given then 'DEFAULT'
 will be used.
 .TP
+[\fB!\fR] \fB--eq \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+is equal to (or not !) count. No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--lt \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+is less than count (or not !). No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--gt \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+is greater than count (or not !). No other options are considered if
+this is not true.
+.TP
 [\fB!\fR] \fB--set\fR
 This will add the source address of the packet to the list. If the
 source address is already in the list, this will update the existing
--- extensions/libipt_recent.c.orig	2006-02-20 15:42:26.000000000 +0000
+++ extensions/libipt_recent.c	2006-02-20 15:45:23.000000000 +0000
@@ -33,6 +33,9 @@
 	{ .name = "name",     .has_arg = 1, .flag = 0, .val = 208 },
 	{ .name = "rsource",  .has_arg = 0, .flag = 0, .val = 209 },
 	{ .name = "rdest",    .has_arg = 0, .flag = 0, .val = 210 },
+	{ .name = "eq",       .has_arg = 1, .flag = 0, .val = 211 },
+	{ .name = "lt",       .has_arg = 1, .flag = 0, .val = 212 },
+	{ .name = "gt",       .has_arg = 1, .flag = 0, .val = 213 },
 	{ .name = 0,          .has_arg = 0, .flag = 0, .val = 0   }
 };
 
@@ -42,6 +45,11 @@
 {
 	printf(
 "recent v%s options:\n"
+"    PRECONDITIONS               Only one precondition may be set.\n"
+"[!] --eq n                      Only match if the precondition is true that there are n items in the table\n"
+"[!] --lt n                      Only match if the precondition is true that there are less than n items in the table\n"
+"[!] --gt n                      Only match if the precondition is true that there are more than n items in the table\n"
+"                                The precondtion may be followed by:\n"
 "[!] --set                       Add source address to list, always matches.\n"
 "[!] --rcheck                    Match if source address in list.\n"
 "[!] --update                    Match if source address in list, also update last-seen time.\n"
@@ -155,6 +163,30 @@
 			info->side = IPT_RECENT_DEST;
 			break;
 
+		case 211:
+			if (info->check_count) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--lt', `--gt' "
+                                        " or `--eq' may be set");
+			info->check_count |= IPT_RECENT_EQ | (invert?IPT_RECENT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 212:
+			if (info->check_count) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--lt', `--gt' "
+                                        " or `--eq' may be set");
+			info->check_count |= IPT_RECENT_LT | (invert?IPT_RECENT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 213:
+			if (info->check_count) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--lt', `--gt' "
+                                        " or `--eq' may be set");
+			info->check_count |= IPT_RECENT_GT | (invert?IPT_RECENT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
 		default:
 			return 0;
 	}
@@ -181,10 +213,18 @@
 {
 	struct ipt_recent_info *info = (struct ipt_recent_info *)match->data;
 
+	printf("recent: ");
+
+	/* handle the precondition first */
+	if (info->check_count) {
+	  if ((info->check_count & IPT_RECENT_EQ)==IPT_RECENT_EQ) printf("%s %d ",(info->check_count & IPT_RECENT_INVERT)?"!=":"==",info->entry_count);
+	  else if (info->check_count & IPT_RECENT_LT) printf("%s %d ",(info->check_count & IPT_RECENT_INVERT)?">=":"<",info->entry_count);
+	  else if (info->check_count & IPT_RECENT_GT) printf("%s %d ",(info->check_count & IPT_RECENT_INVERT)?"<=":">",info->entry_count);
+        }
+
 	if (info->invert)
 		fputc('!', stdout);
 
-	printf("recent: ");
 	if(info->check_set & IPT_RECENT_SET) printf("SET ");
 	if(info->check_set & IPT_RECENT_CHECK) printf("CHECK ");
 	if(info->check_set & IPT_RECENT_UPDATE) printf("UPDATE ");
@@ -203,6 +243,12 @@
 {
 	struct ipt_recent_info *info = (struct ipt_recent_info *)match->data;
 
+	/* handle the precondition first */
+	if (info->check_count) {
+	  if (info->check_count & IPT_RECENT_INVERT) printf("! ");
+	  if ((info->check_count & IPT_RECENT_EQ)==IPT_RECENT_EQ) printf("--eq %d ",info->entry_count);
+	  else if (info->check_count & IPT_RECENT_LT) printf("--lt %d ",info->entry_count);
+	}
 	if (info->invert)
 		printf("! ");
 

[-- Attachment #3: ipt_recent.patch --]
[-- Type: text/x-patch, Size: 5736 bytes --]

--- ./net/ipv4/netfilter/ipt_recent.c.nolimit	2006-02-15 16:34:20.000000000 +0000
+++ ./net/ipv4/netfilter/ipt_recent.c	2006-02-20 16:06:17.000000000 +0000
@@ -5,7 +5,6 @@
 /* This software is distributed under the terms of the GPL, Version 2 */
 /* This copyright does not cover user programs that use kernel services
  * by normal system calls. */
-
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/proc_fs.h>
@@ -70,7 +69,11 @@
 /* Structure of our linked list of tables of recent lists. */
 struct recent_ip_tables {
 	char name[IPT_RECENT_NAME_LEN];
+	/* number of entries in list *table */
+	int entry_count;
+	/* number of reference to this structure from iptables rules */
 	int count;
+	/* an index increased with each operation which maps time_info[x].position to a position in table */
 	int time_pos;
 	struct recent_ip_list *table;
 	struct recent_ip_tables *next;
@@ -139,6 +142,7 @@
 	curr_table = (struct recent_ip_tables*) data;
 
 	spin_lock_bh(&curr_table->list_lock);
+	len += sprintf(buffer+len,"count=%d\n",curr_table->entry_count);
 	for(count = 0; count < ip_list_tot; count++) {
 		if(!curr_table->table[count].addr) continue;
 		last_len = len;
@@ -216,6 +220,7 @@
 		used += 5;
 		spin_lock_bh(&curr_table->list_lock);
 		curr_table->time_pos = 0;
+		curr_table->entry_count = 0;
 		for(count = 0; count < ip_list_hash_size; count++) {
 			curr_table->hash_table[count] = -1;
 		}
@@ -297,6 +302,8 @@
 	info->seconds = 0;
 	info->hit_count = 0;
 	info->check_set = check_set;
+	info->entry_count = 0;
+	info->check_count = 0;
 	info->invert = 0;
 	info->side = IPT_RECENT_SOURCE;
 	strncpy(info->name,curr_table->name,IPT_RECENT_NAME_LEN);
@@ -350,6 +357,10 @@
  * --hitcount -- Option to --rcheck/--update, only match if seen hitcount times
  *   -- matchinfo->hit_count
  * --seconds and --hitcount can be combined
+ * --lt -- Option to see if there are less-than so-many entries in the table altogether
+ * --gt -- Option to see if there are greater-than so-many entries in the table altogether
+ * --eq -- Option to see if there are exactly so-many entries in the table altogether
+ * --lt and --gt and --eq and be combined with --seconds but NOT --hitcount. 
  */
 static int
 match(const struct sk_buff *skb,
@@ -400,6 +411,18 @@
 	/* Table with this name not found, match impossible */
 	if(!curr_table) { return ans; }
 
+	/* If table content count doesn't match, then further checks are unneccessary */
+	if(info->check_count) {
+	  int uninvert=(0==(info->check_count & IPT_RECENT_INVERT));
+	  if((info->check_count & IPT_RECENT_EQ) == IPT_RECENT_EQ) {
+	    if(uninvert ^ (curr_table->entry_count==info->entry_count)) return 0;
+	  } else if (info->check_count & IPT_RECENT_LT) { 
+	    if(uninvert ^ (curr_table->entry_count<info->entry_count)) return 0;
+	  } else if (info->check_count & IPT_RECENT_GT) { 
+	    if(uninvert ^ (curr_table->entry_count>info->entry_count)) return 0;
+	  }; 
+	}
+
 	/* Make sure no one is changing the list while we work with it */
 	spin_lock_bh(&curr_table->list_lock);
 
@@ -500,7 +523,27 @@
 
 		/* New item found and IPT_RECENT_SET, so we need to add it */
 		location = time_info[curr_table->time_pos].position;
-		hash_table[r_list[location].hash_entry] = -1;
+		/* 
+		   location is the next slot in r_list that we will use. 
+		   Maybe it is already used and we need to remove it fromthe hash.
+		   If it is used then: 
+		   1) hash_table[r_list[location].hash_entry]=location
+		   Which is to say that anything claiming to have a hash entry
+		   should also be pointed to BY that hash entry.
+		     This only comes up becase by default r_list[x].hash_entry=0
+		   i.e. all items claim to have hash entry 0, so if a real
+		   item has hash slot 0 it gets trashed because of lies.
+		   SO: If r_list[location].hash_entry]==0 we are in warning condition
+		   and need to double check that hash_table reciprocates before we
+		   set the hash_table entry to -1 (it may not really belong)
+		   This would work; 
+		     if (r_list[location].hash_entry!=0 || hash_table[0]==location) hash_table[r_list[location].hash_entry] = -1; 
+		   but we want to combine with a counter for new entries
+		*/
+		/* increase counter if this is a new item */
+		if (hash_table[r_list[location].hash_entry]!=location) curr_table->entry_count++;
+		else hash_table[r_list[location].hash_entry] = -1;
+
 		hash_table[hash_result] = location;
 		memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 		r_list[location].time_pos = curr_table->time_pos;
@@ -633,6 +676,7 @@
 			r_list[location].ttl = 0;
 			memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 			r_list[location].oldest_pkt = 0;
+			curr_table->entry_count--;
 			ans = !info->invert;
 		}
 		spin_unlock_bh(&curr_table->list_lock);
@@ -719,6 +763,7 @@
 	curr_table->next = NULL;
 	curr_table->count = 1;
 	curr_table->time_pos = 0;
+	curr_table->entry_count = 0;
 	strncpy(curr_table->name,info->name,IPT_RECENT_NAME_LEN);
 	curr_table->name[IPT_RECENT_NAME_LEN-1] = '\0';
 
--- include/linux/netfilter_ipv4/ipt_recent.h.nolimit	2006-02-20 10:12:06.000000000 +0000
+++ include/linux/netfilter_ipv4/ipt_recent.h	2006-02-20 11:30:58.000000000 +0000
@@ -10,6 +10,11 @@
 #define IPT_RECENT_REMOVE 8
 #define IPT_RECENT_TTL   16
 
+#define IPT_RECENT_INVERT 1
+#define IPT_RECENT_LT	  2
+#define IPT_RECENT_GT	  4
+#define IPT_RECENT_EQ	  (IPT_RECENT_LT | IPT_RECENT_GT)
+
 #define IPT_RECENT_SOURCE 0
 #define IPT_RECENT_DEST   1
 
@@ -20,6 +25,8 @@
 	u_int32_t   hit_count;
 	u_int8_t    check_set;
 	u_int8_t    invert;
+	u_int8_t    check_count;
+	u_int32_t   entry_count;
 	char        name[IPT_RECENT_NAME_LEN];
 	u_int8_t    side;
 };

^ permalink raw reply	[flat|nested] 16+ messages in thread

* BUG: More ipt_recent queries
  2006-02-17 17:37 ipt_recent fix Amin Azez
  2006-02-20 16:12 ` [patch] ipt_recent Amin Azez
@ 2006-02-21 11:16 ` Amin Azez
  2006-02-21 16:28   ` Amin Azez
  2006-03-04 10:13   ` Patrick McHardy
  2006-03-04 10:10 ` ipt_recent fix Patrick McHardy
  2 siblings, 2 replies; 16+ messages in thread
From: Amin Azez @ 2006-02-21 11:16 UTC (permalink / raw)
  To: netfilter-devel

I'm concerned about ipt_recent where it removes entries from the list.

Surly the move-up-and-close-the-gap while loop will never enter because 
time_info[time_loc].time has just been set to 0 so that this clause of 
the while loop:

   time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time)

will always be false.

Fuller code segment:

location = hash_table[hash_result];
hash_table[r_list[location].hash_entry] = -1;
time_loc = r_list[location].time_pos;
time_info[time_loc].time = 0;
time_info[time_loc].position = location;

while((time_info[(time_loc+1) % ip_list_tot].time <
time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) !=
curr_table->time_pos) {
   time_temp = time_info[time_loc].time;
   time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
   time_info[(time_loc+1)%ip_list_tot].time = time_temp;
   time_temp = time_info[time_loc].position;
   time_info[time_loc].position =
              time_info[(time_loc+1)%ip_list_tot].position;
   time_info[(time_loc+1)%ip_list_tot].position = time_temp;
   r_list[time_info[time_loc].position].time_pos = time_loc;
   r_list[time_info[(time_loc+1)%ip_list_tot].position].time_pos =
              (time_loc+1)%ip_list_tot;
   time_loc = (time_loc+1) % ip_list_tot;
}


I think we should set time_info[time_loc].time = 0; at the end of the 
while loop?

Sam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: BUG: More ipt_recent queries
  2006-02-21 11:16 ` BUG: More ipt_recent queries Amin Azez
@ 2006-02-21 16:28   ` Amin Azez
  2006-03-04 10:13   ` Patrick McHardy
  1 sibling, 0 replies; 16+ messages in thread
From: Amin Azez @ 2006-02-21 16:28 UTC (permalink / raw)
  To: netfilter-devel

Amin Azez wrote:
> I'm concerned about ipt_recent where it removes entries from the list.
> 
> Surly the move-up-and-close-the-gap while loop will never enter because 
> time_info[time_loc].time has just been set to 0 so that this clause of 
> the while loop:
> 
>   time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time)
> 
> will always be false.
> 
> Fuller code segment:
> 
> location = hash_table[hash_result];
> hash_table[r_list[location].hash_entry] = -1;
> time_loc = r_list[location].time_pos;
> time_info[time_loc].time = 0;
> time_info[time_loc].position = location;
> 
> while((time_info[(time_loc+1) % ip_list_tot].time <
> time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) !=
> curr_table->time_pos) {
>   time_temp = time_info[time_loc].time;
>   time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
>   time_info[(time_loc+1)%ip_list_tot].time = time_temp;
>   time_temp = time_info[time_loc].position;
>   time_info[time_loc].position =
>              time_info[(time_loc+1)%ip_list_tot].position;
>   time_info[(time_loc+1)%ip_list_tot].position = time_temp;
>   r_list[time_info[time_loc].position].time_pos = time_loc;
>   r_list[time_info[(time_loc+1)%ip_list_tot].position].time_pos =
>              (time_loc+1)%ip_list_tot;
>   time_loc = (time_loc+1) % ip_list_tot;
> }
> 
> 
> I think we should set time_info[time_loc].time = 0; at the end of the 
> while loop?

This is actually solvable by changeing the while loop condition:
while(((time_loc+1) % ip_list_tot) != curr_table->time_pos) {
...

But appears also to have another problem which is that 
curr_table->time_pos should be decreased (mod ip_list_tot of course) to 
take into account that the last item in the newly shuffled time_info is 
now vacant; otherwise this test:

for I in `seq 1 100` ; \
do echo ${S}1.1.1.$I  > /proc/net/ipt_recent/LIMIT5 ; \
done ; \
echo -1.1.1.5 > /proc/net/ipt_recent/LIMIT5 ; \
echo 1.1.1.200 > /proc/net/ipt_recent/LIMIT5

Leaves 99 values instead of 100 because the slot from 1.1.1.5 was not 
re-used.

Inserting this line after the end of the while loop:
 
curr_table->time_pos=(curr_table->time_pos+ip_list_tot-1)%ip_list_tot;


solves the problem and causes shuffled-to-the-end spare slots in 
time_info to be re-used first. A side effect is that output of 
/proc/net/ipt_recent/* doesn't come out in time order any more; if there 
have been mid-list deletes and then further adds.

We could fix this by having ip_recent_get_info iterate indirectly using 
curr_table->time_info instead of curr_table->table.

Comments?

I guess I'll send one big patch when I get it all closed off.

Sam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch] ipt_recent
  2006-02-20 16:12 ` [patch] ipt_recent Amin Azez
@ 2006-03-04 10:00   ` Patrick McHardy
  2006-03-07 15:48     ` Amin Azez
  0 siblings, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2006-03-04 10:00 UTC (permalink / raw)
  To: Amin Azez; +Cc: netfilter-devel

Amin Azez wrote:
> This patch fixes the previously mentioned bug in ipt_recent and adds:
> 
> --lt n  # check less than n items in list
> --gt n  # checks more than n items in list
> --eq n  # check exactly n items in list
> 
> Which can be prefixed with ! to invert.
>
> --- include/linux/netfilter_ipv4/ipt_recent.h.nolimit	2006-02-20 10:12:06.000000000 +0000
> +++ include/linux/netfilter_ipv4/ipt_recent.h	2006-02-20 11:30:58.000000000 +0000
> @@ -10,6 +10,11 @@
>  #define IPT_RECENT_REMOVE 8
>  #define IPT_RECENT_TTL   16
>  
> +#define IPT_RECENT_INVERT 1
> +#define IPT_RECENT_LT	  2
> +#define IPT_RECENT_GT	  4
> +#define IPT_RECENT_EQ	  (IPT_RECENT_LT | IPT_RECENT_GT)
> +
>  #define IPT_RECENT_SOURCE 0
>  #define IPT_RECENT_DEST   1
>  
> @@ -20,6 +25,8 @@
>  	u_int32_t   hit_count;
>  	u_int8_t    check_set;
>  	u_int8_t    invert;
> +	u_int8_t    check_count;
> +	u_int32_t   entry_count;
>  	char        name[IPT_RECENT_NAME_LEN];
>  	u_int8_t    side;
>  };

Sorry, we can't do that since it breaks userspace compatibility. But I'm
really glad someone finally has the stomach to touch ipt_recent, I'll
review your other patches now.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ipt_recent fix
  2006-02-17 17:37 ipt_recent fix Amin Azez
  2006-02-20 16:12 ` [patch] ipt_recent Amin Azez
  2006-02-21 11:16 ` BUG: More ipt_recent queries Amin Azez
@ 2006-03-04 10:10 ` Patrick McHardy
  2 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2006-03-04 10:10 UTC (permalink / raw)
  To: Amin Azez; +Cc: netfilter-devel

Amin Azez wrote:
> Here is a fix for ipt_recent.
> 
> The problem is that curr_table[x].hash_entry=0 (for all x)
> 
> So all pristine (never used) curr_table entries claim to have hash pos 0.
> 
> So when we add new items to the hash and do:
>  hash_table[r_list[location].hash_entry] = -1;
> to use the oldest (used or unused) location - if it is unused we trash
> anything that really has hash pos 0 already allocated.
> 
> This can be reproduced with this:
> 
> # iptables -A FORWARD -d 1.1.1.1 -m recent --name TEST1 --set
> # G=1 ; while test $G -lt 17 ; do echo $G ; echo 1.1.1.$G> \
>  /proc/net/ipt_recent/TEST1 ; G=$(($G+1)) ; dmesg -c ; cat \
>  /proc/net/ipt_recent/TEST1  | head ; done > x
> 
> the 13th item has hash of 0 and gets trashed when the 14th item is added
> without this patch.
> 
> We should only trash it then if we _know_ it has been used before and
> therefore is REALLY at the position it claims...
> 
> This patch makes sure that items claiming to have a hash entry really do
> have that hash entry before destroying that hash entry.
> 
> This patch also adds counting of the number of hash entries in
> preparation for conditions based on the number of entries in the recent
> table rule; patch for that to follow next week.
> 
> Any comments on this?
> 
> Sam
> 
> 
> ------------------------------------------------------------------------
> 
> --- ./net/ipv4/netfilter/ipt_recent.c.nolimit	2006-02-15 16:34:20.000000000 +0000
> +++ ./net/ipv4/netfilter/ipt_recent.c	2006-02-17 17:23:00.000000000 +0000
> @@ -70,7 +70,11 @@
>  /* Structure of our linked list of tables of recent lists. */
>  struct recent_ip_tables {
>  	char name[IPT_RECENT_NAME_LEN];
> +	/* number of entries in list *table */
> +	int entry_count;
> +	/* number of reference to this structure from iptables rules */
>  	int count;
> +	/* an index increased with each operation which maps time_info[x].position to a position in table */
>  	int time_pos;
>  	struct recent_ip_list *table;
>  	struct recent_ip_tables *next;
> @@ -139,6 +143,7 @@
>  	curr_table = (struct recent_ip_tables*) data;
>  
>  	spin_lock_bh(&curr_table->list_lock);
> +	len += sprintf(buffer+len,"count=%d\n",curr_table->entry_count);
>  	for(count = 0; count < ip_list_tot; count++) {
>  		if(!curr_table->table[count].addr) continue;
>  		last_len = len;

This changes the proc output format, which we shouldn't do without a
good reason to avoid breaking scripts parsing it. Can you seperate
the fix from the patch please and send that? If you think there is
a good reason to change the output format feel free to send a seperate
patch for that.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: BUG: More ipt_recent queries
  2006-02-21 11:16 ` BUG: More ipt_recent queries Amin Azez
  2006-02-21 16:28   ` Amin Azez
@ 2006-03-04 10:13   ` Patrick McHardy
  2006-03-06  3:04     ` Stephen Frost
  1 sibling, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2006-03-04 10:13 UTC (permalink / raw)
  To: Amin Azez; +Cc: sfrost, netfilter-devel

Stephen, can you have a look at this please?

Amin Azez wrote:
> I'm concerned about ipt_recent where it removes entries from the list.
> 
> Surly the move-up-and-close-the-gap while loop will never enter because
> time_info[time_loc].time has just been set to 0 so that this clause of
> the while loop:
> 
>   time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time)
> 
> will always be false.
> 
> Fuller code segment:
> 
> location = hash_table[hash_result];
> hash_table[r_list[location].hash_entry] = -1;
> time_loc = r_list[location].time_pos;
> time_info[time_loc].time = 0;
> time_info[time_loc].position = location;
> 
> while((time_info[(time_loc+1) % ip_list_tot].time <
> time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) !=
> curr_table->time_pos) {
>   time_temp = time_info[time_loc].time;
>   time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
>   time_info[(time_loc+1)%ip_list_tot].time = time_temp;
>   time_temp = time_info[time_loc].position;
>   time_info[time_loc].position =
>              time_info[(time_loc+1)%ip_list_tot].position;
>   time_info[(time_loc+1)%ip_list_tot].position = time_temp;
>   r_list[time_info[time_loc].position].time_pos = time_loc;
>   r_list[time_info[(time_loc+1)%ip_list_tot].position].time_pos =
>              (time_loc+1)%ip_list_tot;
>   time_loc = (time_loc+1) % ip_list_tot;
> }
> 
> 
> I think we should set time_info[time_loc].time = 0; at the end of the
> while loop?
> 
> Sam
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: BUG: More ipt_recent queries
  2006-03-04 10:13   ` Patrick McHardy
@ 2006-03-06  3:04     ` Stephen Frost
  2006-03-07 15:46       ` Amin Azez
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Frost @ 2006-03-06  3:04 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel, Amin Azez

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

* Patrick McHardy (kaber@trash.net) wrote:
> Stephen, can you have a look at this please?

Alright.  I'll try to look over the current ipt_recent state and the
comments I've seen regarding it.  I don't expect to have time to
rewrite it anytime soon though.

	Stephen

> Amin Azez wrote:
> > I'm concerned about ipt_recent where it removes entries from the list.
> > 
> > Surly the move-up-and-close-the-gap while loop will never enter because
> > time_info[time_loc].time has just been set to 0 so that this clause of
> > the while loop:
> > 
> >   time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time)
> > 
> > will always be false.
> > 
> > Fuller code segment:
> > 
> > location = hash_table[hash_result];
> > hash_table[r_list[location].hash_entry] = -1;
> > time_loc = r_list[location].time_pos;
> > time_info[time_loc].time = 0;
> > time_info[time_loc].position = location;
> > 
> > while((time_info[(time_loc+1) % ip_list_tot].time <
> > time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) !=
> > curr_table->time_pos) {
> >   time_temp = time_info[time_loc].time;
> >   time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
> >   time_info[(time_loc+1)%ip_list_tot].time = time_temp;
> >   time_temp = time_info[time_loc].position;
> >   time_info[time_loc].position =
> >              time_info[(time_loc+1)%ip_list_tot].position;
> >   time_info[(time_loc+1)%ip_list_tot].position = time_temp;
> >   r_list[time_info[time_loc].position].time_pos = time_loc;
> >   r_list[time_info[(time_loc+1)%ip_list_tot].position].time_pos =
> >              (time_loc+1)%ip_list_tot;
> >   time_loc = (time_loc+1) % ip_list_tot;
> > }
> > 
> > 
> > I think we should set time_info[time_loc].time = 0; at the end of the
> > while loop?
> > 
> > Sam
> > 
> > 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: BUG: More ipt_recent queries
  2006-03-06  3:04     ` Stephen Frost
@ 2006-03-07 15:46       ` Amin Azez
  0 siblings, 0 replies; 16+ messages in thread
From: Amin Azez @ 2006-03-07 15:46 UTC (permalink / raw)
  To: Stephen Frost; +Cc: netfilter-devel, Patrick McHardy

As for the /proc output change, that was un-intentional, and for 
debugging purposes. I will remove that change, thanks for spotting it.

I'm just finishing testing on new options for ipt_recent, I hope to 
submit a full patch next week including Per's fix.

New options, from the man page:

       [!] --listcount-lt count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-*  specifier)  
is  less
              than  count  (or not !). No other options are considered 
if this
              is not true.

       [!] --listcount-le count
              Requires as a precondition that the number of IP entries 
in  the
              list  (subject  to  the optional --listtime-* specifier) 
is less
              than or equal to count (or not !). No other options are  
consid-
              ered if this is not true.

       [!] --listcount-eq count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-* specifier)  is  
equal
              to  (or not !) count. No other options are considered if 
this is
              not true.

       [!] --listcount-ge count
              Requires as a precondition that the number of IP entries 
in  the
              list (subject to the optional --listtime-* specifier) is 
greater
              than or equal to count (or not !). No other options are  
consid-
              ered if this is not true.

       [!] --listcount-gt count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-* specifier) is 
greater
              than  count  (or not !). No other options are considered 
if this
              is not true.

       Only one --listcount-* option can be specified.

       [!] --listtime-lt seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated less than seconds seconds ago.

       [!] --listtime-le seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated less than or equal to seconds seconds ago.

       [!] --listtime-ge seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated more than or equal to seconds seconds ago.

       [!] --listtime-gt seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated more than seconds seconds ago.

       Only one --listtime-* option can be specified. --listtime-* 
options act
              as select clauses for what to count. The ! negation for  
--list-
              time-*  options merely inverts the comparison, so ! 
--listime-le
              is the same as --listtime-gt

...

              The next example accepts the packet if less than 5 ip  
addresses
              in the list have been updated in the last 60 seconds

              #  iptables  -A FORWARD -m recent --listcount-lt 5 
--listtime-lt
              60 --set -j ACCEPT


Sam

Stephen Frost wrote:

>* Patrick McHardy (kaber@trash.net) wrote:
>  
>
>>Stephen, can you have a look at this please?
>>    
>>
>
>Alright.  I'll try to look over the current ipt_recent state and the
>comments I've seen regarding it.  I don't expect to have time to
>rewrite it anytime soon though.
>
>	Stephen
>
>  
>
>>Amin Azez wrote:
>>    
>>
>>>I'm concerned about ipt_recent where it removes entries from the list.
>>>
>>>Surly the move-up-and-close-the-gap while loop will never enter because
>>>time_info[time_loc].time has just been set to 0 so that this clause of
>>>the while loop:
>>>
>>>  time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time)
>>>
>>>will always be false.
>>>
>>>Fuller code segment:
>>>
>>>location = hash_table[hash_result];
>>>hash_table[r_list[location].hash_entry] = -1;
>>>time_loc = r_list[location].time_pos;
>>>time_info[time_loc].time = 0;
>>>time_info[time_loc].position = location;
>>>
>>>while((time_info[(time_loc+1) % ip_list_tot].time <
>>>time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) !=
>>>curr_table->time_pos) {
>>>  time_temp = time_info[time_loc].time;
>>>  time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
>>>  time_info[(time_loc+1)%ip_list_tot].time = time_temp;
>>>  time_temp = time_info[time_loc].position;
>>>  time_info[time_loc].position =
>>>             time_info[(time_loc+1)%ip_list_tot].position;
>>>  time_info[(time_loc+1)%ip_list_tot].position = time_temp;
>>>  r_list[time_info[time_loc].position].time_pos = time_loc;
>>>  r_list[time_info[(time_loc+1)%ip_list_tot].position].time_pos =
>>>             (time_loc+1)%ip_list_tot;
>>>  time_loc = (time_loc+1) % ip_list_tot;
>>>}
>>>
>>>
>>>I think we should set time_info[time_loc].time = 0; at the end of the
>>>while loop?
>>>
>>>Sam
>>>
>>>
>>>      
>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch] ipt_recent
  2006-03-04 10:00   ` Patrick McHardy
@ 2006-03-07 15:48     ` Amin Azez
  2006-03-08 12:16       ` Patrick McHardy
  0 siblings, 1 reply; 16+ messages in thread
From: Amin Azez @ 2006-03-07 15:48 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

Patrick McHardy wrote:
> Amin Azez wrote:
> 
>>This patch fixes the previously mentioned bug in ipt_recent and adds:
>>
>>--lt n  # check less than n items in list
>>--gt n  # checks more than n items in list
>>--eq n  # check exactly n items in list
>>
>>Which can be prefixed with ! to invert.
>>
>>--- include/linux/netfilter_ipv4/ipt_recent.h.nolimit	2006-02-20 10:12:06.000000000 +0000
>>+++ include/linux/netfilter_ipv4/ipt_recent.h	2006-02-20 11:30:58.000000000 +0000
>>@@ -10,6 +10,11 @@
>> #define IPT_RECENT_REMOVE 8
>> #define IPT_RECENT_TTL   16
>> 
>>+#define IPT_RECENT_INVERT 1
>>+#define IPT_RECENT_LT	  2
>>+#define IPT_RECENT_GT	  4
>>+#define IPT_RECENT_EQ	  (IPT_RECENT_LT | IPT_RECENT_GT)
>>+
>> #define IPT_RECENT_SOURCE 0
>> #define IPT_RECENT_DEST   1
>> 
>>@@ -20,6 +25,8 @@
>> 	u_int32_t   hit_count;
>> 	u_int8_t    check_set;
>> 	u_int8_t    invert;
>>+	u_int8_t    check_count;
>>+	u_int32_t   entry_count;
>> 	char        name[IPT_RECENT_NAME_LEN];
>> 	u_int8_t    side;
>> };
> 
> 
> Sorry, we can't do that since it breaks userspace compatibility. But I'm
> really glad someone finally has the stomach to touch ipt_recent, I'll
> review your other patches now.

I've reworked that functionality significantly in a new patch to send 
next week. I will see if I can find a way to make use of existing 
structures to add the functionality.

I heard tell that ipt_recent needed a maintainer?

Sam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch] ipt_recent
  2006-03-07 15:48     ` Amin Azez
@ 2006-03-08 12:16       ` Patrick McHardy
  2006-03-13 17:47         ` ipt_recent patch Amin Azez
  2006-03-22 12:04         ` [patch] ipt_recent Amin Azez
  0 siblings, 2 replies; 16+ messages in thread
From: Patrick McHardy @ 2006-03-08 12:16 UTC (permalink / raw)
  To: Amin Azez; +Cc: netfilter-devel

Amin Azez wrote:
> Patrick McHardy wrote:
> 
>>> @@ -20,6 +25,8 @@
>>>     u_int32_t   hit_count;
>>>     u_int8_t    check_set;
>>>     u_int8_t    invert;
>>> +    u_int8_t    check_count;
>>> +    u_int32_t   entry_count;
>>>     char        name[IPT_RECENT_NAME_LEN];
>>>     u_int8_t    side;
>>> };
>>
>>
>>
>> Sorry, we can't do that since it breaks userspace compatibility. But I'm
>> really glad someone finally has the stomach to touch ipt_recent, I'll
>> review your other patches now.
> 
> 
> I've reworked that functionality significantly in a new patch to send
> next week. I will see if I can find a way to make use of existing
> structures to add the functionality.

Otherwise you can you versioning as in ipt_MARK and a couple of other
targets.

> 
> I heard tell that ipt_recent needed a maintainer?

Yes, we need someone familiar with the code to review patches, fix
bugs and clean it up.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* ipt_recent patch
  2006-03-08 12:16       ` Patrick McHardy
@ 2006-03-13 17:47         ` Amin Azez
  2006-03-22 14:26           ` Stephen Frost
  2006-03-22 12:04         ` [patch] ipt_recent Amin Azez
  1 sibling, 1 reply; 16+ messages in thread
From: Amin Azez @ 2006-03-13 17:47 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 3869 bytes --]

This patch has some sort of versioning built in so that the new userland 
features are not enabled unless built against a new header file.

If a new kernel module is called with rules by old userland, then it 
refuses to accept the rule.

I don't know if you will like my CPP macros, I could expand them out?

Changes:
* /proc/net/ipt_recent/* is sorted by age, most recent first
* include Per Hedelands patch in message 
<200603062203.k26M3pI5024778@tordmule.bluetail.com>
* fix problem of not moving up list entries after deleting entries which 
meant that sometimes you could never fill the list
* fix problems of items with hash of 0 being erased whenever an empty 
slot is used
* Add these new features:

       [!] --listcount-lt count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-*  specifier)  is 
  less
              than  count  (or not !). No other options are considered 
if this
              is not true.

       [!] --listcount-le count
              Requires as a precondition that the number of IP entries 
in  the
              list  (subject  to  the optional --listtime-* specifier) 
is less
              than or equal to count (or not !). No other options are 
consid-
              ered if this is not true.

       [!] --listcount-eq count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-* specifier)  is 
  equal
              to  (or not !) count. No other options are considered if 
this is
              not true.

       [!] --listcount-ge count
              Requires as a precondition that the number of IP entries 
in  the
              list (subject to the optional --listtime-* specifier) is 
greater
              than or equal to count (or not !). No other options are 
consid-
              ered if this is not true.

       [!] --listcount-gt count
              Requires  as a precondition that the number of IP entries 
in the
              list (subject to the optional --listtime-* specifier) is 
greater
              than  count  (or not !). No other options are considered 
if this
              is not true.

       Only one --listcount-* option can be specified.

       [!] --listtime-lt seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated less than seconds seconds ago.

       [!] --listtime-le seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated less than or equal to seconds seconds ago.

       [!] --listtime-ge seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated more than or equal to seconds seconds ago.

       [!] --listtime-gt seconds
              Affects the --listcount-* so that instead of counting the 
number
              of  items  in  the list, it counts the number of items 
that were
              last updated more than seconds seconds ago.

       Only one --listtime-* option can be specified. --listtime-* 
options act
              as select clauses for what to count. The ! negation for 
--list-
              time-*  options merely inverts the comparison, so ! 
--listime-le
              is the same as --listtime-gt

...

              The next example accepts the packet if less than 5 ip 
addresses
              in the list have been updated in the last 60 seconds

              #  iptables  -A FORWARD -m recent --listcount-lt 5 
--listtime-lt
              60 --set -j ACCEPT

[-- Attachment #2: iptables.recent.patch --]
[-- Type: text/x-patch, Size: 15440 bytes --]

--- extensions/Makefile.orig	2005-07-13 15:25:35.000000000 +0100
+++ extensions/Makefile	2005-07-13 15:25:54.000000000 +0100
@@ -6,8 +6,9 @@
 #
 PF_EXT_SLIB:=ah addrtype comment connlimit connmark conntrack dscp ecn esp hashlimit helper icmp iprange length limit mac mark multiport owner physdev pkttype realm rpc sctp standard state tcp tcpmss tos ttl udp unclean CLASSIFY CONNMARK DNAT DSCP ECN LOG MARK MASQUERADE MIRROR NETMAP NFQUEUE NOTRACK REDIRECT REJECT SAME SNAT TARPIT TCPMSS TOS TRACE TTL ULOG layer7
 PF_EXT_SLIB+=connrate ROUTE time account quota
 PF_EXT_SLIB+=condition
+PF_EXT_SLIB+=recent
 PF_EXT_SLIB+=vlan
 PF6_EXT_SLIB:=eui64 hl icmpv6 length limit mac mark multiport owner physdev standard tcp udp HL LOG NFQUEUE MARK TRACE
  
 # Optionals
--- extensions/libipt_recent.man.orig	2006-03-01 12:39:29.000000000 +0000
+++ extensions/libipt_recent.man	2006-03-02 10:40:53.000000000 +0000
@@ -9,6 +9,63 @@
 Specify the list to use for the commands. If no name is given then 'DEFAULT'
 will be used.
 .TP
+[\fB!\fR] \fB--listcount-lt \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+(subject to the optional --listtime-* specifier)
+is less than count (or not !). No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--listcount-le \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+(subject to the optional --listtime-* specifier)
+is less than or equal to count (or not !). No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--listcount-eq \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+(subject to the optional --listtime-* specifier)
+is equal to (or not !) count. No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--listcount-ge \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+(subject to the optional --listtime-* specifier)
+is greater than or equal to count (or not !). No other options are considered if
+this is not true.
+.TP
+[\fB!\fR] \fB--listcount-gt \ficount\fR
+Requires as a precondition that the number of IP entries in the list
+(subject to the optional --listtime-* specifier)
+is greater than count (or not !). No other options are considered if
+this is not true.
+.TP
+Only one --listcount-* option can be specified.
+.TP
+[\fB!\fR] \fB--listtime-lt \fisecounds\fR
+Affects the --listcount-* so that instead of counting the number of
+items in the list, it counts the number of items that were last updated
+less than \fBseconds\fR seconds ago.
+.TP
+[\fB!\fR] \fB--listtime-le \fisecounds\fR
+Affects the --listcount-* so that instead of counting the number of
+items in the list, it counts the number of items that were last updated
+less than or equal to \fBseconds\fR seconds ago.
+.TP
+[\fB!\fR] \fB--listtime-ge \fisecounds\fR
+Affects the --listcount-* so that instead of counting the number of
+items in the list, it counts the number of items that were last updated
+more than or equal to \fBseconds\fR seconds ago.
+.TP
+[\fB!\fR] \fB--listtime-gt \fisecounds\fR
+Affects the --listcount-* so that instead of counting the number of
+items in the list, it counts the number of items that were last updated
+more than \fBseconds\fR seconds ago.
+.TP
+Only one --listtime-* option can be specified. --listtime-* options act
+as select clauses for what to count. The ! negation for --listtime-*
+options merely inverts the comparison, so \fB! --listime-le\fR is the same as
+\fB--listtime-gt\fR
+.TP
 [\fB!\fR] \fB--set\fR
 This will add the source address of the packet to the list. If the
 source address is already in the list, this will update the existing
@@ -56,6 +113,12 @@
 # iptables -A FORWARD -m recent --name badguy --rcheck --seconds 60 -j DROP
 
 # iptables -A FORWARD -p tcp -i eth0 --dport 139 -m recent --name badguy --set -j DROP
+
+The next example accepts the packet if less than 5 ip addresses in the list have
+been updated in the last 60 seconds
+
+# iptables -A FORWARD -m recent --listcount-lt 5 --listtime-lt 60 --set -j ACCEPT
+
 .P
 Official website (http://snowman.net/projects/ipt_recent/) also has
 some examples of usage.
--- extensions/libipt_recent.c.orig	2006-03-13 17:02:03.000000000 +0000
+++ extensions/libipt_recent.c	2006-03-13 17:19:25.000000000 +0000
@@ -23,17 +23,28 @@
 
 /* Options for this module */
 static struct option opts[] = {
-	{ .name = "set",      .has_arg = 0, .flag = 0, .val = 201 }, 
-	{ .name = "rcheck",   .has_arg = 0, .flag = 0, .val = 202 }, 
-	{ .name = "update",   .has_arg = 0, .flag = 0, .val = 203 },
-	{ .name = "seconds",  .has_arg = 1, .flag = 0, .val = 204 }, 
-	{ .name = "hitcount", .has_arg = 1, .flag = 0, .val = 205 },
-	{ .name = "remove",   .has_arg = 0, .flag = 0, .val = 206 },
-	{ .name = "rttl",     .has_arg = 0, .flag = 0, .val = 207 },
-	{ .name = "name",     .has_arg = 1, .flag = 0, .val = 208 },
-	{ .name = "rsource",  .has_arg = 0, .flag = 0, .val = 209 },
-	{ .name = "rdest",    .has_arg = 0, .flag = 0, .val = 210 },
-	{ .name = 0,          .has_arg = 0, .flag = 0, .val = 0   }
+#ifdef IPT_RECENT_HAS_LIST_OPS
+	{ .name = "set",          .has_arg = 0, .flag = 0, .val = 201 }, 
+	{ .name = "rcheck",       .has_arg = 0, .flag = 0, .val = 202 }, 
+	{ .name = "update",       .has_arg = 0, .flag = 0, .val = 203 },
+	{ .name = "seconds",      .has_arg = 1, .flag = 0, .val = 204 }, 
+	{ .name = "hitcount",     .has_arg = 1, .flag = 0, .val = 205 },
+	{ .name = "remove",       .has_arg = 0, .flag = 0, .val = 206 },
+	{ .name = "rttl",         .has_arg = 0, .flag = 0, .val = 207 },
+	{ .name = "name",         .has_arg = 1, .flag = 0, .val = 208 },
+	{ .name = "rsource",      .has_arg = 0, .flag = 0, .val = 209 },
+	{ .name = "rdest",        .has_arg = 0, .flag = 0, .val = 210 },
+	{ .name = "listcount-lt", .has_arg = 1, .flag = 0, .val = 211 },
+	{ .name = "listcount-le", .has_arg = 1, .flag = 0, .val = 212 },
+	{ .name = "listcount-eq", .has_arg = 1, .flag = 0, .val = 213 },
+	{ .name = "listcount-ge", .has_arg = 1, .flag = 0, .val = 214 },
+	{ .name = "listcount-gt", .has_arg = 1, .flag = 0, .val = 215 },
+	{ .name = "listtime-lt",  .has_arg = 1, .flag = 0, .val = 216 },
+	{ .name = "listtime-le",  .has_arg = 1, .flag = 0, .val = 217 },
+	{ .name = "listtime-ge",  .has_arg = 1, .flag = 0, .val = 218 },
+	{ .name = "listtime-gt",  .has_arg = 1, .flag = 0, .val = 219 },
+#endif
+	{ .name = 0,              .has_arg = 0, .flag = 0, .val = 0   }
 };
 
 /* Function which prints out usage message. */
@@ -42,6 +53,20 @@
 {
 	printf(
 "recent v%s options:\n"
+#ifdef RECENT_HAS_LIST_OPS
+"    PRECONDITIONS               Only one precondition may be set.\n"
+"[!] --listcount-lt n            Only match if there are less than n items in the table\n"
+"[!] --listcount-le n            Only match if there are n items or less in the table\n"
+"[!] --listcount-eq n            Only match if there are n items in the table\n"
+"[!] --listcount-ge n            Only match if there are n items or more in the table\n"
+"[!] --listcount-gt n            Only match if there are more than n items in the table\n"
+
+"[!] --listtimes-lt n            --listcount option only applies to items less than n seconds old\n"
+"[!] --listtimes-le n            --listcount option only applies to items n seconds old or less\n"
+"[!] --listtimes-ge n            --listcount option only applies to items n seconds old or more\n"
+"[!] --listtimes-gt n            --listcount option only applies ti items more than n seconds old\n"
+#endif
+"                                The precondtion may be followed by:\n"
 "[!] --set                       Add source address to list, always matches.\n"
 "[!] --rcheck                    Match if source address in list.\n"
 "[!] --update                    Match if source address in list, also update last-seen time.\n"
@@ -78,6 +103,11 @@
 	 * better be safe, than sorry */
 	info->name[IPT_RECENT_NAME_LEN-1] = '\0';
 	info->side = IPT_RECENT_SOURCE;
+/* If kernel side supports LIST_OPS but we don't set this flag it will
+   presume we are an old client side and refuse the rule */
+#ifdef IPT_RECENT_HAS_LIST_OPS
+	info->check_set |= IPT_RECENT_HAS_LIST_OPS;
+#endif
 }
 
 /* Function which parses command options; returns true if it
@@ -155,6 +185,89 @@
 			info->side = IPT_RECENT_DEST;
 			break;
 
+#ifdef IPT_RECENT_HAS_LIST_OPS
+		case 211: /* listcount-lt */
+			if (info->count_ops & IPT_RECENT_COUNT_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listcount-lt', `--listcount-le' "
+                                        "`--listcount-eq', `--listcount-ge' `listcount-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= IPT_RECENT_COUNT_LT | (invert?IPT_RECENT_COUNT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 212: /* listcount-le */
+			if (info->count_ops & IPT_RECENT_COUNT_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listcount-lt', `--listcount-le' "
+                                        "`--listcount-eq', `--listcount-ge' `listcount-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= IPT_RECENT_COUNT_LE | (invert?IPT_RECENT_COUNT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 213: /* listcount-eq */
+			if (info->count_ops & IPT_RECENT_COUNT_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listcount-lt', `--listcount-le' "
+                                        "`--listcount-eq', `--listcount-ge' `listcount-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= IPT_RECENT_COUNT_EQ | (invert?IPT_RECENT_COUNT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 214: /* listcount-ge */
+			if (info->count_ops & IPT_RECENT_COUNT_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listcount-lt', `--listcount-le' "
+                                        "`--listcount-eq', `--listcount-ge' `listcount-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= IPT_RECENT_COUNT_GE | (invert?IPT_RECENT_COUNT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 215: /* listcount-gt */
+			if (info->count_ops & IPT_RECENT_COUNT_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listcount-lt', `--listcount-le' "
+                                        "`--listcount-eq', `--listcount-ge' `listcount-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= IPT_RECENT_COUNT_GT | (invert?IPT_RECENT_COUNT_INVERT:0);
+			info->entry_count=atoi(optarg);
+			break;
+
+		case 216: /* listtime-lt */
+			if (info->count_ops & IPT_RECENT_TIME_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listtime-lt', `--listtime-le' "
+                                        "`--listtime-eq', `--listtime-ge' `listtime-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= invert?IPT_RECENT_TIME_GE:IPT_RECENT_TIME_LT;
+			info->entry_time=atoi(optarg);
+			break;
+
+		case 217: /* listtime-le */
+			if (info->count_ops & IPT_RECENT_TIME_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listtime-lt', `--listtime-le' "
+                                        "`--listtime-eq', `--listtime-ge' `listtime-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= invert?IPT_RECENT_TIME_GT:IPT_RECENT_TIME_LE;
+			info->entry_time=atoi(optarg);
+			break;
+
+		case 218: /* listtime-ge */
+			if (info->count_ops & IPT_RECENT_TIME_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listtime-lt', `--listtime-le' "
+                                        "`--listtime-eq', `--listtime-ge' `listtime-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= invert?IPT_RECENT_TIME_LT:IPT_RECENT_TIME_GE;
+			info->entry_time=atoi(optarg);
+			break;
+
+		case 219: /* listtime-gt */
+			if (info->count_ops & IPT_RECENT_TIME_OPS) exit_error(PARAMETER_PROBLEM,
+                                        "recent: only one of `--listtime-lt', `--listtime-le' "
+                                        "`--listtime-eq', `--listtime-ge' `listtime-gts'` "
+                                        " or `--eq' may be set");
+			info->count_ops |= invert?IPT_RECENT_TIME_LE:IPT_RECENT_TIME_GT;
+			info->entry_time=atoi(optarg);
+			break;
+#endif
+
 		default:
 			return 0;
 	}
@@ -181,10 +294,35 @@
 {
 	struct ipt_recent_info *info = (struct ipt_recent_info *)match->data;
 
+#ifdef IPT_RECENT_HAS_LIST_OPS
+	printf("recent: ");
+
+	/* handle the precondition first */
+	if (info->count_ops) {
+          if (info->count_ops & IPT_RECENT_COUNT_INVERT) printf("not ");
+	  switch (info->count_ops & IPT_RECENT_COUNT_OPS) {
+	    case IPT_RECENT_COUNT_LT: printf("< ");  break;
+	    case IPT_RECENT_COUNT_LE: printf("<= "); break;
+	    case IPT_RECENT_COUNT_EQ: printf("== "); break;
+	    case IPT_RECENT_COUNT_GE: printf(">= "); break;
+	    case IPT_RECENT_COUNT_GT: printf("> ");  break;
+	    default: printf(" ");
+	  }
+          printf("%d ",info->entry_count);
+	  if (info->count_ops & IPT_RECENT_TIME_OPS) switch (info->count_ops & IPT_RECENT_TIME_OPS) {
+	    case IPT_RECENT_TIME_LT: printf("< ");  break;
+	    case IPT_RECENT_TIME_LE: printf("<= "); break;
+	    case IPT_RECENT_TIME_GE: printf(">= "); break;
+	    case IPT_RECENT_TIME_GT: printf("> ");  break;
+	    default: printf(" ");
+	  }
+          printf("%d ",info->entry_time);
+	}
+#endif
+
 	if (info->invert)
 		fputc('!', stdout);
 
-	printf("recent: ");
 	if(info->check_set & IPT_RECENT_SET) printf("SET ");
 	if(info->check_set & IPT_RECENT_CHECK) printf("CHECK ");
 	if(info->check_set & IPT_RECENT_UPDATE) printf("UPDATE ");
@@ -203,6 +341,31 @@
 {
 	struct ipt_recent_info *info = (struct ipt_recent_info *)match->data;
 
+#ifdef IPT_RECENT_HAS_LIST_OPS
+	/* handle the precondition first */
+	if (info->count_ops) {
+          if (info->count_ops & IPT_RECENT_COUNT_INVERT) printf("! ");
+	  switch (info->count_ops & IPT_RECENT_COUNT_OPS) {
+	    case IPT_RECENT_COUNT_LT: printf("--listcount-lt ");  break;
+	    case IPT_RECENT_COUNT_LE: printf("--listcount-le "); break;
+	    case IPT_RECENT_COUNT_EQ: printf("--listcount-eq "); break;
+	    case IPT_RECENT_COUNT_GE: printf("--listcount-ge "); break;
+	    case IPT_RECENT_COUNT_GT: printf("--listcount-gt ");  break;
+	    default: printf(" ");
+	  }
+          printf("%d ",info->entry_count);
+
+	  if (info->count_ops & IPT_RECENT_TIME_OPS) switch (info->count_ops & IPT_RECENT_TIME_OPS) {
+	    case IPT_RECENT_TIME_LT: printf("--listtime-lt ");  break;
+	    case IPT_RECENT_TIME_LE: printf("--listtime-le "); break;
+	    case IPT_RECENT_TIME_GE: printf("--listtime-ge "); break;
+	    case IPT_RECENT_TIME_GT: printf("--listtime-gt ");  break;
+	    default: printf(" ");
+	  } 
+          printf("%d ",info->entry_time);
+	}
+#endif
+
 	if (info->invert)
 		printf("! ");
 

[-- Attachment #3: kernel_ipt_recent.patch --]
[-- Type: text/x-patch, Size: 19620 bytes --]

--- net/ipv4/netfilter/ipt_recent.c.nolimit	2006-02-15 16:34:20.000000000 +0000
+++ net/ipv4/netfilter/ipt_recent.c	2006-03-13 17:26:41.000000000 +0000
@@ -1,11 +1,12 @@
 /* Kernel module to check if the source address has been seen recently. */
 /* Copyright 2002-2003, Stephen Frost, 2.5.x port by laforge@netfilter.org */
 /* Author: Stephen Frost <sfrost@snowman.net> */
+/* Copyright 2006 ufomechanic.net <azez@ufomechanic.net> 
+   for --listcount and --listtime enhancements */
 /* Project Page: http://snowman.net/projects/ipt_recent/ */
 /* This software is distributed under the terms of the GPL, Version 2 */
 /* This copyright does not cover user programs that use kernel services
  * by normal system calls. */
-
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/proc_fs.h>
@@ -70,7 +71,11 @@
 /* Structure of our linked list of tables of recent lists. */
 struct recent_ip_tables {
 	char name[IPT_RECENT_NAME_LEN];
+	/* number of entries in list *table */
+	int entry_count;
+	/* number of reference to this structure from iptables rules */
 	int count;
+	/* an index increased with each operation which maps time_info[x].position to a position in table */
 	int time_pos;
 	struct recent_ip_list *table;
 	struct recent_ip_tables *next;
@@ -131,7 +136,7 @@
 
 static int ip_recent_get_info(char *buffer, char **start, off_t offset, int length, int *eof, void *data)
 {
-	int len = 0, count, last_len = 0, pkt_count;
+	int len = 0, index, time, count, last_len = 0, pkt_count;
 	off_t pos = 0;
 	off_t begin = 0;
 	struct recent_ip_tables *curr_table;
@@ -139,22 +144,27 @@
 	curr_table = (struct recent_ip_tables*) data;
 
 	spin_lock_bh(&curr_table->list_lock);
+	time=(curr_table->time_pos+ip_list_tot-1) % ip_list_tot;
 	for(count = 0; count < ip_list_tot; count++) {
-		if(!curr_table->table[count].addr) continue;
+		/* The next slot to be re-used is the oldest slot, so start there... */
+		index=curr_table->time_info[time].position;
 		last_len = len;
-		len += sprintf(buffer+len,"src=%u.%u.%u.%u ",NIPQUAD(curr_table->table[count].addr));
-		len += sprintf(buffer+len,"ttl: %u ",curr_table->table[count].ttl);
-		len += sprintf(buffer+len,"last_seen: %lu ",curr_table->table[count].last_seen);
-		len += sprintf(buffer+len,"oldest_pkt: %u ",curr_table->table[count].oldest_pkt);
-		len += sprintf(buffer+len,"last_pkts: %lu",curr_table->table[count].last_pkts[0]);
-		for(pkt_count = 1; pkt_count < ip_pkt_list_tot; pkt_count++) {
-			if(!curr_table->table[count].last_pkts[pkt_count]) break;
-			len += sprintf(buffer+len,", %lu",curr_table->table[count].last_pkts[pkt_count]);
-		}
-		len += sprintf(buffer+len,"\n");
-		pos = begin + len;
-		if(pos < offset) { len = 0; begin = pos; }
-		if(pos > offset + length) { len = last_len; break; }
+		if(curr_table->table[index].addr) {
+		  len += sprintf(buffer+len,"src=%u.%u.%u.%u ",NIPQUAD(curr_table->table[index].addr));
+		  len += sprintf(buffer+len,"ttl: %u ",curr_table->table[index].ttl);
+		  len += sprintf(buffer+len,"last_seen: %lu ",curr_table->table[index].last_seen);
+		  len += sprintf(buffer+len,"oldest_pkt: %u ",curr_table->table[index].oldest_pkt);
+		  len += sprintf(buffer+len,"last_pkts: %lu",curr_table->table[index].last_pkts[0]);
+		  for(pkt_count = 1; pkt_count < ip_pkt_list_tot; pkt_count++) {
+			if(!curr_table->table[index].last_pkts[pkt_count]) break;
+			len += sprintf(buffer+len,", %lu",curr_table->table[index].last_pkts[pkt_count]);
+		  }
+		  len += sprintf(buffer+len,"\n");
+		  pos = begin + len;
+		  if(pos < offset) { len = 0; begin = pos; }
+		  if(pos > offset + length) { len = last_len; break; }
+		}
+		time=(time-1+ip_list_tot) % ip_list_tot;
 	}
 
 	*start = buffer + (offset - begin);
@@ -216,6 +226,7 @@
 		used += 5;
 		spin_lock_bh(&curr_table->list_lock);
 		curr_table->time_pos = 0;
+		curr_table->entry_count = 0;
 		for(count = 0; count < ip_list_hash_size; count++) {
 			curr_table->hash_table[count] = -1;
 		}
@@ -297,6 +308,9 @@
 	info->seconds = 0;
 	info->hit_count = 0;
 	info->check_set = check_set;
+	info->entry_count = 0;
+	info->entry_time = 0;
+	info->count_ops = 0;
 	info->invert = 0;
 	info->side = IPT_RECENT_SOURCE;
 	strncpy(info->name,curr_table->name,IPT_RECENT_NAME_LEN);
@@ -350,6 +364,16 @@
  * --hitcount -- Option to --rcheck/--update, only match if seen hitcount times
  *   -- matchinfo->hit_count
  * --seconds and --hitcount can be combined
+ * --listcount-lt -- Options to see if how many entries in the table altogether
+ * --listcount-le
+ * --listcount-eq
+ * --listcount-ge
+ * --listcount-gt
+ * --listtime-lt -- change --listcount to refer to how many entries in the table we
+ *                  updated less than so many seconds ago
+ * --listtime-le
+ * --listtime-ge
+ * --listtime-gt
  */
 static int
 match(const struct sk_buff *skb,
@@ -403,7 +427,132 @@
 	/* Make sure no one is changing the list while we work with it */
 	spin_lock_bh(&curr_table->list_lock);
 
+	/* Get jiffies now in case they changed while we were waiting for a lock */
+	now = jiffies;
+
 	r_list = curr_table->table;
+
+	/* We seek: count of items matching time constraint.
+
+	   We don't actually want to count the number of items meeting
+	   the time constraints because it is a waste of time, we already
+	   have an index in time order, we can easily check that there
+	   are enough in the index.
+
+	     location = time_info[curr_table->time_pos].position;
+           references the next slot to be used, and which therefore is the 
+	   oldest slot to have been used if it has been used.
+
+	     start = time_info[(curr_table->time_pos + ip_list_tot -1) % ip_list_tot].position;
+	   refers to the most recent slot to be used if any have been used
+
+	   (or if curr_table->entry_count > 0)
+	     end = time_info[(curr_table->time_pos + ip_list_tot
+	       - curr_table->entry_count) % ip_list_tot ].position;
+	   will always show the oldest item if entry_count > 0
+
+	    SO, if we want to check that there are n items in the list that
+	   are < or <= to t then we go to position (END+n-1) and compare that.
+	   If it is < or <= t then so are the old items 
+	    If we want to check that there are n items > or >= t then we
+	   check item START-n+1 and compare that, if it is < or <= t then so
+	   are the items after it.
+
+	   Here are the full cases we cover. We don't do time= because its not
+	   very useful and difficult to optimize
+
+	   time >  t, count >  c : itemcount > c  && [start-c]>t      ; there are enough, and past the limitmatches
+	   time >  t, count >= c : itemcount >= c && [start-c+1]>t    ; there are enough and the limit matches
+	   time >  t, count =  c : itemcount >= c && [start-c+1]>t    ; there are at least enough and the limit matches
+	                                      && (c<=itemcount || !([start-c]>t)) ; others, if any, do not match
+	   time >  t, count <= c : itemcount <= c || !([start-c]>t)   ; either there aren't too many anyway, or the one just out of count doesn't match
+	   time >  t, count <  c : itemcount < c  || !([start-c+1]>t) ; either there aren't too many anyway, or the one just out of count doesn't match
+
+
+	   time >= t, count >  c : itemcount > c && [start-c]>=t      ; there are enough past the limit
+	   time >= t, count >= c : itemcount >=c && [start-c+1]>=t    ; there are enough and the limit matches
+	   time >= t, count =  c : itemcount >=c && [start-c+1]>=t    ; there are at least enough and the limit matches
+	                                      && (c<=itemcount || !([start-c]>=t)) ; others, if any, do not match
+	   time >= t, count <= c : itemcount <=c || !([start-c]>=t)   ; either there aren't too many anyway, or the one just out of count doesn't match
+	   time >= t, count <  c : itemcount < c || !([start-c+1]>=t) ; either there aren't too many anyway, or the one just out of count doesn't match
+
+
+	   time <= t, count >  c : itemcount > c  && [end+c]<=t        ; Make sure that at least 1 more than c matches
+	   time <= t, count >= c : itemcount >= c && [end+c-1]<=t      ; Make sure that at least c items match
+	   time <= t, count =  c : itemcount >= c && [end+c-1]<=t      ; there are at least enough and the limit matches
+	                                      && (c<=itemcount) || !([end+c]<=t)) ; others, if any, do not match
+	   time <= t, count <= c : itemcount <=c || !([end+c]<=t)      ; either there aren't too many anyway, or the one out of count doesn't match
+	   time <= t, count <  c : itemcount < c || !([end+c-1]<=t)      ; either there aren't too many anyway, or the one out of count doesn't match
+
+
+	   time <  t, count >  c : itemcount > c  && [end+c]<t         ; Make sure that at least 1 more than c matches
+	   time <  t, count >= c : itemcount >= c && [end+c-1]<t       ; Make sure that at least c items match
+	   time <  t, count =  c : itemcount >= c && [end+c-1]<t       ; there are at least enough and the limit matches
+	                                      && (c<=itemcount) || !([end+c]<t)) ; others, if any, do not match
+	   time <  t, count <= c : itemcount <=c || !([end+c]<t)       ; either there aren't too many anyway, or the one out of count doesn't match
+	   time <  t, count <  c : itemcount < c || !([end+c-1]<t)       ; either there aren't too many anyway, or the one out of count doesn't match
+	BTW the last rule is the same as this:
+	   time <  t, count <  c : !(itemcount >= c && ([end+c-1]<t)       ; either there aren't too many anyway, or the one out of count doesn't match
+
+
+	We can spot the pattern in thse and reduce it to one set of factorized rules.
+	This will sadden some people */
+
+	/* General parameterized count rule */
+	#define COUNT_(tablelist,timecheck,TIME_OP,COUNT_CHECK,limit) ( (tablelist->entry_count COUNT_CHECK \
+	                   ((!timecheck) || TIME_OP(now,tablelist->time_info[(limit+ip_list_tot)%ip_list_tot].time+(unsigned long)(timecheck)*HZ))) )
+
+	#define COUNT_GT(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) COUNT_(table,timecheck,TIME_OP, > countcheck &&,limit DIR_OP countcheck)
+	#define COUNT_GE(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) COUNT_(table,timecheck,TIME_OP,>= countcheck &&,limit DIR_OP countcheck - DIR_OP 1)
+	#define COUNT_EQ(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) COUNT_GE(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) \
+	                                                               && ! COUNT_GT(table,timecheck,TIME_OP,countcheck,limit,DIR_OP)
+	#define COUNT_LE(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) COUNT_(table,timecheck,TIME_OP,<= countcheck || !,limit DIR_OP countcheck)
+	#define COUNT_LT(table,timecheck,TIME_OP,countcheck,limit,DIR_OP) COUNT_(table,timecheck,TIME_OP, < countcheck || !,limit DIR_OP countcheck - DIR_OP 1)
+
+	/* and one rule to bind them.  Macros were made for this.
+	   Inline functions won't be efficient here because we would have to pass a function pointer or something */
+	#define COUNT(COUNT_FUNC,TIME_OP,table,timecheck,countcheck) (\
+	 (((TIME_OP)==IPT_RECENT_TIME_GT)?(COUNT_FUNC(table,timecheck,time_after,     countcheck,table->time_pos-table->entry_count,+)):\
+	  ((TIME_OP)==IPT_RECENT_TIME_GE)?(COUNT_FUNC(table,timecheck,time_after_eq,  countcheck,table->time_pos-table->entry_count,+)):\
+	  ((TIME_OP)==IPT_RECENT_TIME_LE)?(COUNT_FUNC(table,timecheck,time_before_eq, countcheck,table->time_pos-1,-)):\
+	  ((TIME_OP)==IPT_RECENT_TIME_LT)?(COUNT_FUNC(table,timecheck,time_before,    countcheck,table->time_pos-1,-)):\
+	  ((TIME_OP)==0                 )?(COUNT_FUNC(table,0,        time_after,     countcheck,table->time_pos-1,-)):\
+	  0) \
+	 )
+
+	/* we use do..while(0) so that the acceptable cases can "break" away from the 
+	   exit block which saves repeating the exit block for each case */
+	if(info->count_ops) do {
+	  int invert=(0!=(info->count_ops & IPT_RECENT_COUNT_INVERT));
+#ifdef DEBUG
+	if (debug) printk(KERN_INFO RECENT_NAME " match invert=%d count_ops=%x count=%d time=%d\n",invert,info->count_ops,info->entry_count,info->entry_time);
+#endif
+
+	  /* of course limit and over_limit may be invalid if c<count */
+	  /* For LT or LE limit=end+c-1; over_limit=limit+1; */
+	  /* treat GT, GE or EQ with same limits limit=start-c+1; over_limit=limit-1; */
+	  if ((info->count_ops & IPT_RECENT_COUNT_OPS)==IPT_RECENT_COUNT_LT) {
+	    if (invert ^ COUNT(COUNT_LT,info->count_ops & IPT_RECENT_TIME_OPS,curr_table,info->entry_time,info->entry_count)) break;
+	  } else if ((info->count_ops & IPT_RECENT_COUNT_OPS)==IPT_RECENT_COUNT_LE) {
+	    if (invert ^ COUNT(COUNT_LE,info->count_ops & IPT_RECENT_TIME_OPS,curr_table,info->entry_time,info->entry_count)) break;
+	  } else if ((info->count_ops & IPT_RECENT_COUNT_OPS)==IPT_RECENT_COUNT_EQ) {
+	    if (invert ^ COUNT(COUNT_EQ,info->count_ops & IPT_RECENT_TIME_OPS,curr_table,info->entry_time,info->entry_count)) break;
+	  } else if ((info->count_ops & IPT_RECENT_COUNT_OPS)== IPT_RECENT_COUNT_GE) {
+	    if (invert ^ COUNT(COUNT_GE,info->count_ops & IPT_RECENT_TIME_OPS,curr_table,info->entry_time,info->entry_count)) break;
+	  } else if ((info->count_ops & IPT_RECENT_COUNT_OPS)== IPT_RECENT_COUNT_GT) {
+	    if (invert ^ COUNT(COUNT_GT,info->count_ops & IPT_RECENT_TIME_OPS,curr_table,info->entry_time,info->entry_count)) break;
+	  }
+	  /* If we get here then a definite comparison failed */
+          spin_unlock_bh(&curr_table->list_lock);
+	  return 0;
+	} while(0);
+
+	/* this is the next slot to use. 
+	   If it has never been used then 0 is the oldest slot
+	   If it has been used then it is the oldest slot. */
+	location = time_info[curr_table->time_pos].position;
+	
+
 	if(info->side == IPT_RECENT_DEST) addr = skb->nh.iph->daddr; else addr = skb->nh.iph->saddr;
 
 	if(!addr) { 
@@ -418,8 +567,6 @@
 	if(debug) printk(KERN_INFO RECENT_NAME ": match(): checking table, addr: %u, ttl: %u, orig_ttl: %u\n",addr,ttl,skb->nh.iph->ttl);
 #endif
 
-	/* Get jiffies now in case they changed while we were waiting for a lock */
-	now = jiffies;
 	hash_table = curr_table->hash_table;
 	time_info = curr_table->time_info;
 
@@ -500,7 +647,27 @@
 
 		/* New item found and IPT_RECENT_SET, so we need to add it */
 		location = time_info[curr_table->time_pos].position;
-		hash_table[r_list[location].hash_entry] = -1;
+		/* 
+		   location is the next slot in r_list that we will use. 
+		   Maybe it is already used and we need to remove it from the hash.
+		   If it is used then: 
+		   1) hash_table[r_list[location].hash_entry]=location
+		   Which is to say that anything claiming to have a hash entry
+		   should also be pointed to BY that hash entry.
+		     This only comes up becase by default r_list[x].hash_entry=0
+		   i.e. all items claim to have hash entry 0, so if a real
+		   item has hash slot 0 it gets trashed because of lies.
+		   SO: If r_list[location].hash_entry]==0 we are in warning condition
+		   and need to double check that hash_table reciprocates before we
+		   set the hash_table entry to -1 (it may not really belong)
+		   This would work; 
+		     if (r_list[location].hash_entry!=0 || hash_table[0]==location) hash_table[r_list[location].hash_entry] = -1; 
+		   but we want to combine with a counter for new entries
+		*/
+		/* increase counter if this is a new item */
+		if (hash_table[r_list[location].hash_entry]!=location) curr_table->entry_count++;
+		else hash_table[r_list[location].hash_entry] = -1;
+
 		hash_table[hash_result] = location;
 		memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 		r_list[location].time_pos = curr_table->time_pos;
@@ -587,7 +754,7 @@
 #endif
 			/* Check if this is part of a collision chain */
 			while(hash_table[(orig_hash_result+1) % ip_list_hash_size] != -1) {
-				orig_hash_result++;
+				orig_hash_result = (orig_hash_result+1) % ip_list_hash_size;
 				if(hash_func(r_list[hash_table[orig_hash_result]].addr,ip_list_hash_size) == hash_result) {
 					/* Found collision chain, how deep does this rabbit hole go? */
 #ifdef DEBUG
@@ -616,7 +783,10 @@
 			time_loc = r_list[location].time_pos;
 			time_info[time_loc].time = 0;
 			time_info[time_loc].position = location;
-			while((time_info[(time_loc+1) % ip_list_tot].time < time_info[time_loc].time) && ((time_loc+1) % ip_list_tot) != curr_table->time_pos) {
+			/* close up the gap we made in time_info. 
+			   Perhaps we should work out which is the quickest way around the ring buffer 
+			   and close up the gap the quickest way. We should use memmove while we are at it. */
+			while(((time_loc+1) % ip_list_tot) != curr_table->time_pos) {
 				time_temp = time_info[time_loc].time;
 				time_info[time_loc].time = time_info[(time_loc+1)%ip_list_tot].time;
 				time_info[(time_loc+1)%ip_list_tot].time = time_temp;
@@ -633,6 +803,9 @@
 			r_list[location].ttl = 0;
 			memset(r_list[location].last_pkts,0,ip_pkt_list_tot*sizeof(u_int32_t));
 			r_list[location].oldest_pkt = 0;
+			curr_table->entry_count--;
+			/* back off next-free-slot time_pos to the free slot we just bubbled to the end */
+			curr_table->time_pos=(curr_table->time_pos+ip_list_tot-1)%ip_list_tot;
 			ans = !info->invert;
 		}
 		spin_unlock_bh(&curr_table->list_lock);
@@ -669,6 +842,11 @@
 
 	if (matchsize != IPT_ALIGN(sizeof(struct ipt_recent_info))) return 0;
 
+	if(! (info->check_set & IPT_RECENT_HAS_LIST_OPS)) {
+		printk(KERN_INFO RECENT_NAME ": supports LIST_OPS but user iptables does not!\n");
+		return 0;
+	}
+
 	/* seconds and hit_count only valid for CHECK/UPDATE */
 	if(info->check_set & IPT_RECENT_SET) { flag++; if(info->seconds || info->hit_count) return 0; }
 	if(info->check_set & IPT_RECENT_REMOVE) { flag++; if(info->seconds || info->hit_count) return 0; }
@@ -719,6 +897,7 @@
 	curr_table->next = NULL;
 	curr_table->count = 1;
 	curr_table->time_pos = 0;
+	curr_table->entry_count = 0;
 	strncpy(curr_table->name,info->name,IPT_RECENT_NAME_LEN);
 	curr_table->name[IPT_RECENT_NAME_LEN-1] = '\0';
 
--- include/linux/netfilter_ipv4/ipt_recent.h.nolimit	2006-02-20 10:12:06.000000000 +0000
+++ include/linux/netfilter_ipv4/ipt_recent.h	2006-03-13 17:11:08.000000000 +0000
@@ -2,13 +2,31 @@
 #define _IPT_RECENT_H
 
 #define RECENT_NAME	"ipt_recent"
-#define RECENT_VER	"v0.3.1"
+#define RECENT_VER	"v0.3.2"
 
 #define IPT_RECENT_CHECK  1
 #define IPT_RECENT_SET    2
 #define IPT_RECENT_UPDATE 4
 #define IPT_RECENT_REMOVE 8
 #define IPT_RECENT_TTL   16
+#define IPT_RECENT_HAS_LIST_OPS 32
+
+#ifdef IPT_RECENT_HAS_LIST_OPS
+#define IPT_RECENT_COUNT_INVERT 1 << 0
+#define IPT_RECENT_COUNT_LT	1 << 1
+#define IPT_RECENT_COUNT_EQ	1 << 2
+#define IPT_RECENT_COUNT_GT	1 << 3
+#define IPT_RECENT_COUNT_LE	(IPT_RECENT_COUNT_LT | IPT_RECENT_COUNT_EQ)
+#define IPT_RECENT_COUNT_GE	(IPT_RECENT_COUNT_GT | IPT_RECENT_COUNT_EQ)
+#define IPT_RECENT_COUNT_OPS	(IPT_RECENT_COUNT_LT | IPT_RECENT_COUNT_EQ | IPT_RECENT_COUNT_GT)
+
+#define IPT_RECENT_TIME_LT	1 << 4
+#define IPT_RECENT_TIME_EQ	1 << 5 /* But we don't implement checks for == itself */
+#define IPT_RECENT_TIME_GT	1 << 6
+#define IPT_RECENT_TIME_LE	(IPT_RECENT_TIME_LT | IPT_RECENT_TIME_EQ)
+#define IPT_RECENT_TIME_GE	(IPT_RECENT_TIME_GT | IPT_RECENT_TIME_EQ)
+#define IPT_RECENT_TIME_OPS	(IPT_RECENT_TIME_LT | IPT_RECENT_TIME_EQ | IPT_RECENT_TIME_GT)
+#endif
 
 #define IPT_RECENT_SOURCE 0
 #define IPT_RECENT_DEST   1
@@ -20,6 +38,11 @@
 	u_int32_t   hit_count;
 	u_int8_t    check_set;
 	u_int8_t    invert;
+#ifdef IPT_RECENT_HAS_LIST_OPS
+	u_int8_t    count_ops;
+	u_int32_t   entry_count;
+	u_int32_t   entry_time;
+#endif
 	char        name[IPT_RECENT_NAME_LEN];
 	u_int8_t    side;
 };

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [patch] ipt_recent
  2006-03-08 12:16       ` Patrick McHardy
  2006-03-13 17:47         ` ipt_recent patch Amin Azez
@ 2006-03-22 12:04         ` Amin Azez
  1 sibling, 0 replies; 16+ messages in thread
From: Amin Azez @ 2006-03-22 12:04 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

Patrick McHardy wrote:
...
> 
> Otherwise you can you versioning as in ipt_MARK and a couple of other
> targets.

I posted a rework with versioning (although I couldn't see anything in 
ipt_MARK to copy) and posted this on 13th March.

I forgot to Cc you then, and so notify you now.

Sam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ipt_recent patch
  2006-03-13 17:47         ` ipt_recent patch Amin Azez
@ 2006-03-22 14:26           ` Stephen Frost
  2006-05-03  9:32             ` Amin Azez
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Frost @ 2006-03-22 14:26 UTC (permalink / raw)
  To: Amin Azez; +Cc: netfilter-devel, Patrick McHardy

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

* Amin Azez (azez@ufomechanic.net) wrote:
> This patch has some sort of versioning built in so that the new userland 
> features are not enabled unless built against a new header file.
> 
> If a new kernel module is called with rules by old userland, then it 
> refuses to accept the rule.

Works for me.

> Changes:
> * /proc/net/ipt_recent/* is sorted by age, most recent first
> * include Per Hedelands patch in message 
> <200603062203.k26M3pI5024778@tordmule.bluetail.com>
> * fix problem of not moving up list entries after deleting entries which 
> meant that sometimes you could never fill the list
> * fix problems of items with hash of 0 being erased whenever an empty 
> slot is used

These changes look good.  The sorting by age makes sense though isn't
entirely necessary.  The fixes are good, sorry I didn't consider the
case where the hash result was 0. :/

> * Add these new features:

I don't really see the use-case for these new options...  Perhaps if it
was combined with an IP mask of some kind, ie: packets from 5 IPs in the
same /24 in the last 60 seconds, or some such.  That could also be
accomplished by providing a way to tell ipt_recent to look for a mask
instead of individual IPs though.  ie: For this table, consider any IPs
in the same /24 to be the 'same' IP.

Anyway, they don't really affect how the module works as they just add
additional ways to check on the data stored in the tables, so I'm not
strongly against them just don't entirely see the point.

	Thanks,

		Stephen

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ipt_recent patch
  2006-03-22 14:26           ` Stephen Frost
@ 2006-05-03  9:32             ` Amin Azez
  2006-05-03 11:41               ` Amin Azez
  0 siblings, 1 reply; 16+ messages in thread
From: Amin Azez @ 2006-05-03  9:32 UTC (permalink / raw)
  To: Amin Azez, Patrick McHardy, netfilter-devel

Stephen Frost wrote:

> I don't really see the use-case for these new options...  Perhaps if it
> was combined with an IP mask of some kind, ie: packets from 5 IPs in the
> same /24 in the last 60 seconds, or some such.  That could also be
> accomplished by providing a way to tell ipt_recent to look for a mask
> instead of individual IPs though.  ie: For this table, consider any IPs
> in the same /24 to be the 'same' IP.
> 
> Anyway, they don't really affect how the module works as they just add
> additional ways to check on the data stored in the tables, so I'm not
> strongly against them just don't entirely see the point.


I agree that a mask ought also to be supplied, as /32 is equivalent to
the same-ip anyway.

Are you content to make that modification or do you wish me to?
I won't be able to do so for  week or two.

Sam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: ipt_recent patch
  2006-05-03  9:32             ` Amin Azez
@ 2006-05-03 11:41               ` Amin Azez
  0 siblings, 0 replies; 16+ messages in thread
From: Amin Azez @ 2006-05-03 11:41 UTC (permalink / raw)
  To: Amin Azez; +Cc: netfilter-devel, Patrick McHardy

I actually think ipt_recent wants merging with ipt_set, by causing
ipt_set to maintain the insert time and refresh time of each set member.

What do you think? It will require taking the design behind ipt_recents
combine hash/list structure but I would suggest using a linked list
instead of an array to hold the time-ordered list.

Sam

Amin Azez wrote:
> Stephen Frost wrote:
>
>   
>> I don't really see the use-case for these new options...  Perhaps if it
>> was combined with an IP mask of some kind, ie: packets from 5 IPs in the
>> same /24 in the last 60 seconds, or some such.  That could also be
>> accomplished by providing a way to tell ipt_recent to look for a mask
>> instead of individual IPs though.  ie: For this table, consider any IPs
>> in the same /24 to be the 'same' IP.
>>
>> Anyway, they don't really affect how the module works as they just add
>> additional ways to check on the data stored in the tables, so I'm not
>> strongly against them just don't entirely see the point.
>>     
>
>
> I agree that a mask ought also to be supplied, as /32 is equivalent to
> the same-ip anyway.
>
> Are you content to make that modification or do you wish me to?
> I won't be able to do so for  week or two.
>
> Sam
>   

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2006-05-03 11:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-17 17:37 ipt_recent fix Amin Azez
2006-02-20 16:12 ` [patch] ipt_recent Amin Azez
2006-03-04 10:00   ` Patrick McHardy
2006-03-07 15:48     ` Amin Azez
2006-03-08 12:16       ` Patrick McHardy
2006-03-13 17:47         ` ipt_recent patch Amin Azez
2006-03-22 14:26           ` Stephen Frost
2006-05-03  9:32             ` Amin Azez
2006-05-03 11:41               ` Amin Azez
2006-03-22 12:04         ` [patch] ipt_recent Amin Azez
2006-02-21 11:16 ` BUG: More ipt_recent queries Amin Azez
2006-02-21 16:28   ` Amin Azez
2006-03-04 10:13   ` Patrick McHardy
2006-03-06  3:04     ` Stephen Frost
2006-03-07 15:46       ` Amin Azez
2006-03-04 10:10 ` ipt_recent fix Patrick McHardy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.