Patchwork patch in bugzilla

login
register
mail settings
Submitter Jonh Wendell
Date March 1, 2012, 8:07 p.m.
Message ID <CAO88JF-=PTq2ZXaNWhE6mLKZY96y_fqCqxetNy-6h-Dwv3Ph3Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/144108/
State Rejected
Headers show

Comments

Jonh Wendell - March 1, 2012, 8:07 p.m.
Sorry, guys. In fact, I had tested only libiptc itself, not the iptables binary.

Here is the complete patch, modifying all callers of iptc_append_entry().

I tried it here and it worked fine.

Thanks!

2012/2/29 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Wed, Feb 29, 2012 at 01:50:52PM +0100, Pablo Neira Ayuso wrote:
>> On Tue, Feb 28, 2012 at 04:20:10PM +0100, Jan Engelhardt wrote:
>> > On Tuesday 2012-02-28 13:48, Jonh Wendell wrote:
>> >
>> > >hi, folks. a while ago I filled a bug with a simple patch attached:
>> > >http://bugzilla.netfilter.org/show_bug.cgi?id=768
>> > >
>> > >what's the right place to post things like that? here in this mailing
>> > >list or the bugzilla?
>> >
>> > [Someone still has not pointed the bugzilla automatic notification to
>> > post to the netfilter-devel mailing list :) ]
>> >
>> > Certainly the choice of mailing list makes it more visible. No
>> > objections to the patch.
>>
>> Applied, thanks.
>
> I'm going to revert this patch, it's breaking my iptables script:
>
> # iptables -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT
> iptables: Incompatible with this kernel.
>
> Please, test patches before posting them.
Pablo Neira - March 2, 2012, 4:42 p.m.
On Thu, Mar 01, 2012 at 05:07:57PM -0300, Jonh Wendell wrote:
> diff --git a/include/libiptc/libiptc.h b/include/libiptc/libiptc.h
> index 24cdbdb..b9a42c9 100644
> --- a/include/libiptc/libiptc.h
> +++ b/include/libiptc/libiptc.h
> @@ -74,7 +74,8 @@ int iptc_replace_entry(const xt_chainlabel chain,
>  		       struct xtc_handle *handle);
>  
>  /* Append entry `e' to chain `chain'.  Equivalent to insert with
> -   rulenum = length of chain. */
> +   rulenum = length of chain. Returns the position the entry was
> +   inserted or 0 if an error occurs */
>  int iptc_append_entry(const xt_chainlabel chain,
>  		      const struct ipt_entry *e,
>  		      struct xtc_handle *handle);
> diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
> index b191d5d..8df06d6 100644
> --- a/iptables/ip6tables.c
> +++ b/iptables/ip6tables.c
> @@ -698,7 +698,8 @@ append_entry(const xt_chainlabel chain,
>  			fw->ipv6.dmsk = dmasks[j];
>  			if (verbose)
>  				print_firewall_line(fw, handle);
> -			ret &= ip6tc_append_entry(chain, fw, handle);
> +			if (!ip6tc_append_entry(chain, fw, handle))
> +				ret = 0;
>  		}
>  	}
>

This requires also modifying the libversion numbers for libiptc since the
interface has changed.

IMO, libiptc has always remained an internal library, but it seems
some of you are using it to link your program to it.

Your patch will break other programs made by people like you that use
libiptc.

Jan?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From a5cfd3bd4000fa7152c52ab2ea791efce7d21700 Mon Sep 17 00:00:00 2001
From: Jonh Wendell <jonh.wendell@vexcorp.com>
Date: Thu, 1 Mar 2012 17:04:22 -0300
Subject: [PATCH] libiptc: Returns the position the entry was inserted

---
 include/libiptc/libiptc.h |    3 ++-
 iptables/ip6tables.c      |    3 ++-
 iptables/iptables.c       |    3 ++-
 libiptc/libiptc.c         |    5 +++--
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/libiptc/libiptc.h b/include/libiptc/libiptc.h
index 24cdbdb..b9a42c9 100644
--- a/include/libiptc/libiptc.h
+++ b/include/libiptc/libiptc.h
@@ -74,7 +74,8 @@  int iptc_replace_entry(const xt_chainlabel chain,
 		       struct xtc_handle *handle);
 
 /* Append entry `e' to chain `chain'.  Equivalent to insert with
-   rulenum = length of chain. */
+   rulenum = length of chain. Returns the position the entry was
+   inserted or 0 if an error occurs */
 int iptc_append_entry(const xt_chainlabel chain,
 		      const struct ipt_entry *e,
 		      struct xtc_handle *handle);
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index b191d5d..8df06d6 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -698,7 +698,8 @@  append_entry(const xt_chainlabel chain,
 			fw->ipv6.dmsk = dmasks[j];
 			if (verbose)
 				print_firewall_line(fw, handle);
-			ret &= ip6tc_append_entry(chain, fw, handle);
+			if (!ip6tc_append_entry(chain, fw, handle))
+				ret = 0;
 		}
 	}
 
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 03ac63b..10f30d3 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -700,7 +700,8 @@  append_entry(const xt_chainlabel chain,
 			fw->ip.dmsk.s_addr = dmasks[j].s_addr;
 			if (verbose)
 				print_firewall_line(fw, handle);
-			ret &= iptc_append_entry(chain, fw, handle);
+			if (!iptc_append_entry(chain, fw, handle))
+				ret = 0;
 		}
 	}
 
diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
index 63fcfc2..ddaee12 100644
--- a/libiptc/libiptc.c
+++ b/libiptc/libiptc.c
@@ -1836,7 +1836,8 @@  TC_REPLACE_ENTRY(const IPT_CHAINLABEL chain,
 }
 
 /* Append entry `fw' to chain `chain'.  Equivalent to insert with
-   rulenum = length of chain. */
+   rulenum = length of chain. Returns the position the entry was
+   inserted or 0 if an error occurs */
 int
 TC_APPEND_ENTRY(const IPT_CHAINLABEL chain,
 		const STRUCT_ENTRY *e,
@@ -1872,7 +1873,7 @@  TC_APPEND_ENTRY(const IPT_CHAINLABEL chain,
 
 	set_changed(handle);
 
-	return 1;
+	return c->num_rules;
 }
 
 static inline int
-- 
1.7.5.4