[kakadu@earthlink.net: Re: [PATCH] Double hook registration attempt]

[Brad Chapman]

This is a multi-part message in MIME format.
--------------020408070004010003010807
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Mr. Morris,

      Okeydoke. Here it is. Now it checks for double priority attempts
and uses printk() directly.

Brad

P.S. for those who are interested: I followed Fabrice MARIE's idea
to MIME the patch AND copy it into the e-mail, since he said some
people had problems with the text version.

<snip>

--- linux/net/core/netfilter.c	Mon Jun 11 13:12:07 2001
+++ linux/net/core/netfilter.c	Sat Jul 21 11:30:14 2001
@@ -61,6 +61,20 @@
 	for (i = nf_hooks[reg->pf][reg->hooknum].next; 
 	     i != &nf_hooks[reg->pf][reg->hooknum]; 
 	     i = i->next) {
+		/* Detect an attempt to register the same hook twice */
+		if (reg->hook = ((struct nf_hook_ops *)i)->hook) {
+			printk(KERN_WARNING "OUCH! Double hook registration attempt for hooknum/pf %d/%d!\n",
+					reg->hooknum, reg->pf);
+			br_write_unlock_bh(BR_NETPROTO_LOCK);
+			return -1;
+		}
+		/* Detect an attempt to register hooks with the same priority as another */
+		if (reg->priority == ((struct nf_hook_ops *)i)->priority) {
+			printk(KERN_WARNING "OUCH! Double priority registration attempt for hooknum/pf %d/%d!\n",
+					reg->hooknum, reg->pf);
+			br_write_unlock_bh(BR_NETPROTO_LOCK);
+			return -1;
+		}
 		if (reg->priority < ((struct nf_hook_ops *)i)->priority)
 			break;
 	}
@@ -72,6 +86,13 @@
 void nf_unregister_hook(struct nf_hook_ops *reg)
 {
 	br_write_lock_bh(BR_NETPROTO_LOCK);
+	/* Detect an attempt to unregister the same hook twice */
+	if (!list_inlist(&reg->list, reg)) {
+		printk(KERN_WARNING "OUCH! Double hook unregistration attempt for hooknum/pf %d/%d!\n",
+				reg->hooknum, reg->pf);
+		br_write_unlock_bh(BR_NETPROTO_LOCK);
+		return;
+	}
 	list_del(&reg->list);
 	br_write_unlock_bh(BR_NETPROTO_LOCK);
 }

<snip>

James Morris wrote:

> On Sat, 21 Jul 2001, Harald Welte wrote:
> 
>> - shouldn't we print a warning message, instead of a debug message?
>> - isn't it better to generally (or additionally) check if a priority is
>>   registered twice at one hook?
>> 
>> I'd welcome any comments on that.  I'd be happy to make a fix ready, and
>> submit it towards the kernel..
> 
> 
> I think it would be a good idea.
> 
> - Jame



--------------020408070004010003010807
Content-Type: text/plain;
 name="doublehook.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="doublehook.patch"

--- linux/net/core/netfilter.c	Mon Jun 11 13:12:07 2001
+++ linux/net/core/netfilter.c	Sat Jul 21 11:30:14 2001
@@ -61,6 +61,20 @@
 	for (i = nf_hooks[reg->pf][reg->hooknum].next; 
 	     i != &nf_hooks[reg->pf][reg->hooknum]; 
 	     i = i->next) {
+		/* Detect an attempt to register the same hook twice */
+		if (reg->hook = ((struct nf_hook_ops *)i)->hook) {
+			printk(KERN_WARNING "OUCH! Double hook registration attempt for hooknum/pf %d/%d!\n",
+					reg->hooknum, reg->pf);
+			br_write_unlock_bh(BR_NETPROTO_LOCK);
+			return -1;
+		}
+		/* Detect an attempt to register hooks with the same priority as another */
+		if (reg->priority == ((struct nf_hook_ops *)i)->priority) {
+			printk(KERN_WARNING "OUCH! Double priority registration attempt for hooknum/pf %d/%d!\n",
+					reg->hooknum, reg->pf);
+			br_write_unlock_bh(BR_NETPROTO_LOCK);
+			return -1;
+		}
 		if (reg->priority < ((struct nf_hook_ops *)i)->priority)
 			break;
 	}
@@ -72,6 +86,13 @@
 void nf_unregister_hook(struct nf_hook_ops *reg)
 {
 	br_write_lock_bh(BR_NETPROTO_LOCK);
+	/* Detect an attempt to unregister the same hook twice */
+	if (!list_inlist(&reg->list, reg)) {
+		printk(KERN_WARNING "OUCH! Double hook unregistration attempt for hooknum/pf %d/%d!\n",
+				reg->hooknum, reg->pf);
+		br_write_unlock_bh(BR_NETPROTO_LOCK);
+		return;
+	}
 	list_del(&reg->list);
 	br_write_unlock_bh(BR_NETPROTO_LOCK);
 }

--------------020408070004010003010807--