netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* (unknown), 
@ 2009-10-29 18:11 Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 1/3] build: restore --disable-ipv6 functionality on system w/o v6 headers Jan Engelhardt
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jan Engelhardt @ 2009-10-29 18:11 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel


Hi,


here are three commits that fix bugzilla entries and/or other
problems encountered. There are also two extra commits prepended
without any changes, which only provide missing log entries for
already-merged commits.


The following changes since commit 7fa7329fc972513021131416dbd9d535141bd2ea:
  Jan Engelhardt (1):
        iprange: roll address parsing into a loop

are available in the git repository at:

  git://dev.medozas.de/iptables master

Jan Engelhardt (4):
      iprange: do accept non-ranges for xt_iprange v1 (log)
      iprange: warn on reverse range (log)
      libiptc: fix wrong maptype of base chain counters on restore
      iptables: fix undersized deletion mask creation

Olaf Rempel (1):
      build: restore --disable-ipv6 functionality on system w/o v6 headers

 ip6tables.c       |   14 ++++++++------
 iptables.c        |   14 ++++++++------
 libiptc/libiptc.c |    2 +-
 xtables.c         |    3 ++-
 4 files changed, 19 insertions(+), 14 deletions(-)

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

* [PATCH 1/3] build: restore --disable-ipv6 functionality on system w/o v6 headers
  2009-10-29 18:11 (unknown), Jan Engelhardt
@ 2009-10-29 18:11 ` Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 2/3] libiptc: fix wrong maptype of base chain counters on restore Jan Engelhardt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jan Engelhardt @ 2009-10-29 18:11 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

From: Olaf Rempel <razzor@kopf-tisch.de>

Commit 332e4acc (iptables: accept multiple IP address specifications
for -s, d) broke the --disable-ipv6 configure option.

> ./.libs/libxtables.so: undefined reference to `in6addr_any'

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 xtables.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/xtables.c b/xtables.c
index 4520124..bda49f8 100644
--- a/xtables.c
+++ b/xtables.c
@@ -1484,6 +1484,7 @@ void
 xtables_ip6parse_multiple(const char *name, struct in6_addr **addrpp,
 		      struct in6_addr **maskpp, unsigned int *naddrs)
 {
+	static const struct in6_addr zero_addr;
 	struct in6_addr *addrp;
 	char buf[256], *p;
 	unsigned int len, i, j, n, count = 1;
@@ -1526,7 +1527,7 @@ xtables_ip6parse_multiple(const char *name, struct in6_addr **addrpp,
 		memcpy(*maskpp + i, addrp, sizeof(*addrp));
 
 		/* if a null mask is given, the name is ignored, like in "any/0" */
-		if (memcmp(*maskpp + i, &in6addr_any, sizeof(in6addr_any)) == 0)
+		if (memcmp(*maskpp + i, &zero_addr, sizeof(zero_addr)) == 0)
 			strcpy(buf, "::");
 
 		addrp = ip6parse_hostnetwork(buf, &n);
-- 
1.6.5.1


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

* [PATCH 2/3] libiptc: fix wrong maptype of base chain counters on restore
  2009-10-29 18:11 (unknown), Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 1/3] build: restore --disable-ipv6 functionality on system w/o v6 headers Jan Engelhardt
@ 2009-10-29 18:11 ` Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 3/3] iptables: fix undersized deletion mask creation Jan Engelhardt
  2009-10-29 22:26 ` Patrick McHardy
  3 siblings, 0 replies; 8+ messages in thread
From: Jan Engelhardt @ 2009-10-29 18:11 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

When a ruleset that does not reset any chain policies/counters, such as

	*filter
	COMMIT

is sourced by iptables-restore, the previous policy and counters
(i.e. the ones read from the kernel) are reused. The counter skew
offsetting is wrong however, causing the read value to be readded to
the kernel value. This manifests itself in practice by the counter
value almost doubling everytime iptables-restore is called.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 libiptc/libiptc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
index 670acf5..7a9c742 100644
--- a/libiptc/libiptc.c
+++ b/libiptc/libiptc.c
@@ -829,7 +829,7 @@ static int __iptcc_p_del_policy(struct xtc_handle *h, unsigned int num)
 
 		/* save counter and counter_map information */
 		h->chain_iterator_cur->counter_map.maptype =
-						COUNTER_MAP_NORMAL_MAP;
+						COUNTER_MAP_ZEROED;
 		h->chain_iterator_cur->counter_map.mappos = num-1;
 		memcpy(&h->chain_iterator_cur->counters, &pr->entry->counters,
 			sizeof(h->chain_iterator_cur->counters));
-- 
1.6.5.1


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

* [PATCH 3/3] iptables: fix undersized deletion mask creation
  2009-10-29 18:11 (unknown), Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 1/3] build: restore --disable-ipv6 functionality on system w/o v6 headers Jan Engelhardt
  2009-10-29 18:11 ` [PATCH 2/3] libiptc: fix wrong maptype of base chain counters on restore Jan Engelhardt
@ 2009-10-29 18:11 ` Jan Engelhardt
  2009-10-29 22:55   ` Patrick McHardy
  2009-10-29 22:26 ` Patrick McHardy
  3 siblings, 1 reply; 8+ messages in thread
From: Jan Engelhardt @ 2009-10-29 18:11 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

The mask created for the -D rulespec is simply too small.
xtables_targets points to whatever target has last been loaded, so
xtables_targets->size is quite almost wrong, as we need to use the
size of the target for the specific rule that is about to be deleted.

This bug existed ever since iptables history is tracked, and requires
certain circumstances to be visible, where the deletion operation is
one. Furthermore, multiple userspace target extensions must have been
loaded, and a target B whose .size is smaller than the target A of
the rule we are about to delete must have been loaded more recently
than target A. The minimal testcase is (rule 60007 gets wrongly
removed)

	*nat
	-F
	-X
	-A POSTROUTING -p udp -j SNAT --to 192.168.1.1:60007
	-A POSTROUTING -p udp -j SNAT --to 192.168.1.1:60008
	-A POSTROUTING -p udp -j CONNMARK --set-mark 0
	-D POSTROUTING -p udp -j SNAT --to 192.168.1.1:60008
	COMMIT

References: http://bugzilla.netfilter.org/show_bug.cgi?id=606
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 ip6tables.c |   14 ++++++++------
 iptables.c  |   14 ++++++++------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/ip6tables.c b/ip6tables.c
index 8f653e8..53a1a5d 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -803,7 +803,8 @@ insert_entry(const ip6t_chainlabel chain,
 }
 
 static unsigned char *
-make_delete_mask(struct xtables_rule_match *matches)
+make_delete_mask(struct xtables_rule_match *matches,
+		 const struct xtables_target *target)
 {
 	/* Establish mask for comparison */
 	unsigned int size;
@@ -816,7 +817,7 @@ make_delete_mask(struct xtables_rule_match *matches)
 
 	mask = xtables_calloc(1, size
 			 + IP6T_ALIGN(sizeof(struct ip6t_entry_target))
-			 + xtables_targets->size);
+			 + target->size);
 
 	memset(mask, 0xFF, sizeof(struct ip6t_entry));
 	mptr = mask + sizeof(struct ip6t_entry);
@@ -830,7 +831,7 @@ make_delete_mask(struct xtables_rule_match *matches)
 
 	memset(mptr, 0xFF,
 	       IP6T_ALIGN(sizeof(struct ip6t_entry_target))
-	       + xtables_targets->userspacesize);
+	       + target->userspacesize);
 
 	return mask;
 }
@@ -846,13 +847,14 @@ delete_entry(const ip6t_chainlabel chain,
 	     const struct in6_addr dmasks[],
 	     int verbose,
 	     struct ip6tc_handle *handle,
-	     struct xtables_rule_match *matches)
+	     struct xtables_rule_match *matches,
+	     const struct xtables_target *target)
 {
 	unsigned int i, j;
 	int ret = 1;
 	unsigned char *mask;
 
-	mask = make_delete_mask(matches);
+	mask = make_delete_mask(matches, target);
 	for (i = 0; i < nsaddrs; i++) {
 		fw->ipv6.src = saddrs[i];
 		fw->ipv6.smsk = smasks[i];
@@ -1938,7 +1940,7 @@ int do_command6(int argc, char *argv[], char **table, struct ip6tc_handle **hand
 				   nsaddrs, saddrs, smasks,
 				   ndaddrs, daddrs, dmasks,
 				   options&OPT_VERBOSE,
-				   *handle, matches);
+				   *handle, matches, target);
 		break;
 	case CMD_DELETE_NUM:
 		ret = ip6tc_delete_num_entry(chain, rulenum - 1, *handle);
diff --git a/iptables.c b/iptables.c
index 7228721..1160171 100644
--- a/iptables.c
+++ b/iptables.c
@@ -805,7 +805,8 @@ insert_entry(const ipt_chainlabel chain,
 }
 
 static unsigned char *
-make_delete_mask(struct xtables_rule_match *matches)
+make_delete_mask(struct xtables_rule_match *matches,
+		 const struct xtables_target *target)
 {
 	/* Establish mask for comparison */
 	unsigned int size;
@@ -818,7 +819,7 @@ make_delete_mask(struct xtables_rule_match *matches)
 
 	mask = xtables_calloc(1, size
 			 + IPT_ALIGN(sizeof(struct ipt_entry_target))
-			 + xtables_targets->size);
+			 + target->size);
 
 	memset(mask, 0xFF, sizeof(struct ipt_entry));
 	mptr = mask + sizeof(struct ipt_entry);
@@ -832,7 +833,7 @@ make_delete_mask(struct xtables_rule_match *matches)
 
 	memset(mptr, 0xFF,
 	       IPT_ALIGN(sizeof(struct ipt_entry_target))
-	       + xtables_targets->userspacesize);
+	       + target->userspacesize);
 
 	return mask;
 }
@@ -848,13 +849,14 @@ delete_entry(const ipt_chainlabel chain,
 	     const struct in_addr dmasks[],
 	     int verbose,
 	     struct iptc_handle *handle,
-	     struct xtables_rule_match *matches)
+	     struct xtables_rule_match *matches,
+	     const struct xtables_target *target)
 {
 	unsigned int i, j;
 	int ret = 1;
 	unsigned char *mask;
 
-	mask = make_delete_mask(matches);
+	mask = make_delete_mask(matches, target);
 	for (i = 0; i < nsaddrs; i++) {
 		fw->ip.src.s_addr = saddrs[i].s_addr;
 		fw->ip.smsk.s_addr = smasks[i].s_addr;
@@ -1979,7 +1981,7 @@ int do_command(int argc, char *argv[], char **table, struct iptc_handle **handle
 				   nsaddrs, saddrs, smasks,
 				   ndaddrs, daddrs, dmasks,
 				   options&OPT_VERBOSE,
-				   *handle, matches);
+				   *handle, matches, target);
 		break;
 	case CMD_DELETE_NUM:
 		ret = iptc_delete_num_entry(chain, rulenum - 1, *handle);
-- 
1.6.5.1


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

* Re:
  2009-10-29 18:11 (unknown), Jan Engelhardt
                   ` (2 preceding siblings ...)
  2009-10-29 18:11 ` [PATCH 3/3] iptables: fix undersized deletion mask creation Jan Engelhardt
@ 2009-10-29 22:26 ` Patrick McHardy
  2009-10-29 22:51   ` Re: Jan Engelhardt
  3 siblings, 1 reply; 8+ messages in thread
From: Patrick McHardy @ 2009-10-29 22:26 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

Jan Engelhardt wrote:
> here are three commits that fix bugzilla entries and/or other
> problems encountered. There are also two extra commits prepended
> without any changes, which only provide missing log entries for
> already-merged commits.

Just to clarify before I apply this - how does adding changelog
entries afterwards work? Am I correct to assume that this won't
affect this history of the tree and existing clones?


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

* Re:
  2009-10-29 22:26 ` Patrick McHardy
@ 2009-10-29 22:51   ` Jan Engelhardt
  2009-10-29 22:55     ` Re: Patrick McHardy
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Engelhardt @ 2009-10-29 22:51 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

On Thursday 2009-10-29 23:26, Patrick McHardy wrote:

>Jan Engelhardt wrote:
>> here are three commits that fix bugzilla entries and/or other
>> problems encountered. There are also two extra commits prepended
>> without any changes, which only provide missing log entries for
>> already-merged commits.
>
>Just to clarify before I apply this - how does adding changelog
>entries afterwards work? Am I correct to assume that this won't
>affect this history of the tree and existing clones?

I just used `git commit --allow-empty -e` to record a plain commit on 
top, just without any change in the tree object. Take a look in 
git-forest/gitk if in doubt ;-)

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

* Re:
  2009-10-29 22:51   ` Re: Jan Engelhardt
@ 2009-10-29 22:55     ` Patrick McHardy
  0 siblings, 0 replies; 8+ messages in thread
From: Patrick McHardy @ 2009-10-29 22:55 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

Jan Engelhardt wrote:
> On Thursday 2009-10-29 23:26, Patrick McHardy wrote:
> 
>> Jan Engelhardt wrote:
>>> here are three commits that fix bugzilla entries and/or other
>>> problems encountered. There are also two extra commits prepended
>>> without any changes, which only provide missing log entries for
>>> already-merged commits.
>> Just to clarify before I apply this - how does adding changelog
>> entries afterwards work? Am I correct to assume that this won't
>> affect this history of the tree and existing clones?
> 
> I just used `git commit --allow-empty -e` to record a plain commit on 
> top, just without any change in the tree object. Take a look in 
> git-forest/gitk if in doubt ;-)

Nice. Pulled and pushed out again, thanks.


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

* Re: [PATCH 3/3] iptables: fix undersized deletion mask creation
  2009-10-29 18:11 ` [PATCH 3/3] iptables: fix undersized deletion mask creation Jan Engelhardt
@ 2009-10-29 22:55   ` Patrick McHardy
  0 siblings, 0 replies; 8+ messages in thread
From: Patrick McHardy @ 2009-10-29 22:55 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

Jan Engelhardt wrote:
> The mask created for the -D rulespec is simply too small.
> xtables_targets points to whatever target has last been loaded, so
> xtables_targets->size is quite almost wrong, as we need to use the
> size of the target for the specific rule that is about to be deleted.
> 
> This bug existed ever since iptables history is tracked, and requires
> certain circumstances to be visible, where the deletion operation is
> one. Furthermore, multiple userspace target extensions must have been
> loaded, and a target B whose .size is smaller than the target A of
> the rule we are about to delete must have been loaded more recently
> than target A. The minimal testcase is (rule 60007 gets wrongly
> removed)
> 
> 	*nat
> 	-F
> 	-X
> 	-A POSTROUTING -p udp -j SNAT --to 192.168.1.1:60007
> 	-A POSTROUTING -p udp -j SNAT --to 192.168.1.1:60008
> 	-A POSTROUTING -p udp -j CONNMARK --set-mark 0
> 	-D POSTROUTING -p udp -j SNAT --to 192.168.1.1:60008
> 	COMMIT
> 
> References: http://bugzilla.netfilter.org/show_bug.cgi?id=606

Very nice catch, thanks Jan.

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

end of thread, other threads:[~2009-10-29 22:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-29 18:11 (unknown), Jan Engelhardt
2009-10-29 18:11 ` [PATCH 1/3] build: restore --disable-ipv6 functionality on system w/o v6 headers Jan Engelhardt
2009-10-29 18:11 ` [PATCH 2/3] libiptc: fix wrong maptype of base chain counters on restore Jan Engelhardt
2009-10-29 18:11 ` [PATCH 3/3] iptables: fix undersized deletion mask creation Jan Engelhardt
2009-10-29 22:55   ` Patrick McHardy
2009-10-29 22:26 ` Patrick McHardy
2009-10-29 22:51   ` Re: Jan Engelhardt
2009-10-29 22:55     ` Re: Patrick McHardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).