iptables throws unknown error - suspecting 32/64 compat issue

iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

Hi,


the following command gives an error:

	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW

output is:

	iptables: Unknown error 4294967295

As mentioned in the topic, I suspect it is due to 32-bit iptables not 
coping correctly with the 64-bit kernel (sometimes, patches to fix these 
are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
with their latest(?) kernel 2.6.20-1.2986.al3.3smp.


	Jan
-- 

iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

Hi,


the following command gives an error:

	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW

output is:

	iptables: Unknown error 4294967295

As mentioned in the topic, I suspect it is due to 32-bit iptables not 
coping correctly with the 64-bit kernel (sometimes, patches to fix these 
are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
with their latest(?) kernel 2.6.20-1.2986.al3.3smp.


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 08:27, Jan Engelhardt wrote:
>Hi,
>
>
>the following command gives an error:
>
>	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW
>
>output is:
>
>	iptables: Unknown error 4294967295
>
>As mentioned in the topic, I suspect it is due to 32-bit iptables not 
>coping correctly with the 64-bit kernel (sometimes, patches to fix these 
>are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
>with their latest(?) kernel 2.6.20-1.2986.al3.3smp.

And the following cmd oopsed it:

	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
	-j sshcheck;

Output:

Unable to handle kernel NULL pointer dereference
tsk->{mm,active_mm}->context = 000000000000078d
tsk->{mm,active_mm}->pgd = fffff80055a48000
              \|/ ____ \|/
              "@'/ .. \`@"
              /_| \__/ |_\
                 \__U_/
iptables(9865): Oops [#1]
TSTATE: 0000009911009606 TPC: 000000000052853c TNPC: 0000000000528540 Y:
00000000    Not tainted
TPC: <atomic_sub_ret+0xc/0x30>
g0: fffff8001fdf4000 g1: 000000001029a340
g2: 00000000000000c8 g3: 0000000000000000
g4: fffff8007fbe6f00 g5: fffff80000e80000
g6: fffff8001fdf4000 g7: 0000000000000005
o0: 0000000000000001 o1: 0000000000000010
o2: 0000000010284810 o3: 0000000000000050
o4: 0000000000000048 o5: 0000000000000640
sp: fffff8001fdf70a1 ret_pc: 000000001029a350
RPC: <hashlimit_destroy+0x18/0xc0 [xt_hashlimit]>
l0: 000000001028f000 l1: 0000000000000000
l2: 0000000000000040 l3: 0000000000000000
l4: 0000000000030bbc l5: 000000000002f690
l6: 0000000000031600 l7: 000000000002ca50
i0: 0000000000000000 i1: 000000001029d400
i2: 0000000010284800 i3: fffff8001a0fb4a0
i4: 0000000000000002 i5: 0000000000000006
i6: fffff8001fdf7161 i7: 000000001022a2a8
I7: <compat_do_ipt_set_ctl+0x7d4/0xa04 [ip_tables]>
Caller[000000001022a2a8]: compat_do_ipt_set_ctl+0x7d4/0xa04 [ip_tables]
Caller[00000000006155b4]: compat_nf_sockopt+0x130/0x16c
Caller[0000000000615628]: compat_nf_setsockopt+0x24/0x30
Caller[0000000000623e74]: compat_ip_setsockopt+0xac/0xc0
Caller[00000000005f10e4]: compat_sock_common_setsockopt+0x48/0x54
Caller[0000000000609ba0]: compat_sys_setsockopt+0x4b4/0x4ec
Caller[0000000000406c14]: linux_sparc_syscall32+0x3c/0x40
Caller[0000000000019340]: 0x19348
Instruction DUMP: 81c3e008  01000000  8143e003 <c2024000> 8e204008  cfe25001
80a04007  1247fffc  8e21c008



	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 08:27, Jan Engelhardt wrote:
>Hi,
>
>
>the following command gives an error:
>
>	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW
>
>output is:
>
>	iptables: Unknown error 4294967295
>
>As mentioned in the topic, I suspect it is due to 32-bit iptables not 
>coping correctly with the 64-bit kernel (sometimes, patches to fix these 
>are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
>with their latest(?) kernel 2.6.20-1.2986.al3.3smp.

And the following cmd oopsed it:

	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
	-j sshcheck;

Output:

Unable to handle kernel NULL pointer dereference
tsk->{mm,active_mm}->context = 000000000000078d
tsk->{mm,active_mm}->pgd = fffff80055a48000
              \|/ ____ \|/
              "@'/ .. \`@"
              /_| \__/ |_\
                 \__U_/
iptables(9865): Oops [#1]
TSTATE: 0000009911009606 TPC: 000000000052853c TNPC: 0000000000528540 Y:
00000000    Not tainted
TPC: <atomic_sub_ret+0xc/0x30>
g0: fffff8001fdf4000 g1: 000000001029a340
g2: 00000000000000c8 g3: 0000000000000000
g4: fffff8007fbe6f00 g5: fffff80000e80000
g6: fffff8001fdf4000 g7: 0000000000000005
o0: 0000000000000001 o1: 0000000000000010
o2: 0000000010284810 o3: 0000000000000050
o4: 0000000000000048 o5: 0000000000000640
sp: fffff8001fdf70a1 ret_pc: 000000001029a350
RPC: <hashlimit_destroy+0x18/0xc0 [xt_hashlimit]>
l0: 000000001028f000 l1: 0000000000000000
l2: 0000000000000040 l3: 0000000000000000
l4: 0000000000030bbc l5: 000000000002f690
l6: 0000000000031600 l7: 000000000002ca50
i0: 0000000000000000 i1: 000000001029d400
i2: 0000000010284800 i3: fffff8001a0fb4a0
i4: 0000000000000002 i5: 0000000000000006
i6: fffff8001fdf7161 i7: 000000001022a2a8
I7: <compat_do_ipt_set_ctl+0x7d4/0xa04 [ip_tables]>
Caller[000000001022a2a8]: compat_do_ipt_set_ctl+0x7d4/0xa04 [ip_tables]
Caller[00000000006155b4]: compat_nf_sockopt+0x130/0x16c
Caller[0000000000615628]: compat_nf_setsockopt+0x24/0x30
Caller[0000000000623e74]: compat_ip_setsockopt+0xac/0xc0
Caller[00000000005f10e4]: compat_sock_common_setsockopt+0x48/0x54
Caller[0000000000609ba0]: compat_sys_setsockopt+0x4b4/0x4ec
Caller[0000000000406c14]: linux_sparc_syscall32+0x3c/0x40
Caller[0000000000019340]: 0x19348
Instruction DUMP: 81c3e008  01000000  8143e003 <c2024000> 8e204008  cfe25001
80a04007  1247fffc  8e21c008



	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[David Miller]

From: Jan Engelhardt <jengelh@linux01.gwdg.de>
Date: Thu, 10 May 2007 08:34:07 +0200 (MEST)

> Unable to handle kernel NULL pointer dereference
> tsk->{mm,active_mm}->context = 000000000000078d
> tsk->{mm,active_mm}->pgd = fffff80055a48000
>               \|/ ____ \|/
>               "@'/ .. \`@"
>               /_| \__/ |_\
>                  \__U_/
> iptables(9865): Oops [#1]
> TSTATE: 0000009911009606 TPC: 000000000052853c TNPC: 0000000000528540 Y:
> 00000000    Not tainted
> TPC: <atomic_sub_ret+0xc/0x30>
> g0: fffff8001fdf4000 g1: 000000001029a340
> g2: 00000000000000c8 g3: 0000000000000000
> g4: fffff8007fbe6f00 g5: fffff80000e80000
> g6: fffff8001fdf4000 g7: 0000000000000005
> o0: 0000000000000001 o1: 0000000000000010
> o2: 0000000010284810 o3: 0000000000000050
> o4: 0000000000000048 o5: 0000000000000640
> sp: fffff8001fdf70a1 ret_pc: 000000001029a350
> RPC: <hashlimit_destroy+0x18/0xc0 [xt_hashlimit]>

in hashlimit_destroy(), r->hinfo is NULL, and this NULL pointer
is being passed to atomic_sub_ret() via htable_put()'s

	if (atomic_dec_and_test(&hinfo->use))

call.

It would be interesting if the current 2.6.21 vanilla kernel
gives you the same problem, please built that and retest because
I believe some compat issues were fixed between 2.6.20 and
2.6.21

Thanks.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[David Miller]

From: Jan Engelhardt <jengelh@linux01.gwdg.de>
Date: Thu, 10 May 2007 08:34:07 +0200 (MEST)

> Unable to handle kernel NULL pointer dereference
> tsk->{mm,active_mm}->context = 000000000000078d
> tsk->{mm,active_mm}->pgd = fffff80055a48000
>               \|/ ____ \|/
>               "@'/ .. \`@"
>               /_| \__/ |_\
>                  \__U_/
> iptables(9865): Oops [#1]
> TSTATE: 0000009911009606 TPC: 000000000052853c TNPC: 0000000000528540 Y:
> 00000000    Not tainted
> TPC: <atomic_sub_ret+0xc/0x30>
> g0: fffff8001fdf4000 g1: 000000001029a340
> g2: 00000000000000c8 g3: 0000000000000000
> g4: fffff8007fbe6f00 g5: fffff80000e80000
> g6: fffff8001fdf4000 g7: 0000000000000005
> o0: 0000000000000001 o1: 0000000000000010
> o2: 0000000010284810 o3: 0000000000000050
> o4: 0000000000000048 o5: 0000000000000640
> sp: fffff8001fdf70a1 ret_pc: 000000001029a350
> RPC: <hashlimit_destroy+0x18/0xc0 [xt_hashlimit]>

in hashlimit_destroy(), r->hinfo is NULL, and this NULL pointer
is being passed to atomic_sub_ret() via htable_put()'s

	if (atomic_dec_and_test(&hinfo->use))

call.

It would be interesting if the current 2.6.21 vanilla kernel
gives you the same problem, please built that and retest because
I believe some compat issues were fixed between 2.6.20 and
2.6.21

Thanks.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Jan Engelhardt wrote:
> Hi,
> 
> 
> the following command gives an error:
> 
> 	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW
> 
> output is:
> 
> 	iptables: Unknown error 4294967295
> 
> As mentioned in the topic, I suspect it is due to 32-bit iptables not 
> coping correctly with the 64-bit kernel (sometimes, patches to fix these 
> are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
> with their latest(?) kernel 2.6.20-1.2986.al3.3smp.


The conntrack match is missing compat support, I've queued this patch
to fix it.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2652 bytes --]

[NETFILTER]: xt_conntrack: add compat support

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit ba8991494e1522be10d764b174fc4e3744c99655
tree 85d5cf3861566aa38ecb2e091be987ecfeb17655
parent 1797736897a68f556aef76a6a0963c3e8b1b4950
author Patrick McHardy <kaber@trash.net> Thu, 10 May 2007 14:57:40 +0200
committer Patrick McHardy <kaber@trash.net> Thu, 10 May 2007 14:57:40 +0200

 net/netfilter/xt_conntrack.c |   54 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index f4ea8fe..189ded5 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -134,12 +134,66 @@ static void destroy(const struct xt_match *match, void *matchinfo)
 	nf_ct_l3proto_module_put(match->family);
 }
 
+#ifdef CONFIG_COMPAT
+struct compat_xt_conntrack_info
+{
+	compat_uint_t			statemask;
+	compat_uint_t			statusmask;
+	struct ip_conntrack_old_tuple	tuple[IP_CT_DIR_MAX];
+	struct in_addr			sipmsk[IP_CT_DIR_MAX];
+	struct in_addr			dipmsk[IP_CT_DIR_MAX];
+	compat_ulong_t			expires_min;
+	compat_ulong_t			expires_max;
+	u_int8_t			flags;
+	u_int8_t			invflags;
+};
+
+static void compat_from_user(void *dst, void *src)
+{
+	struct compat_xt_conntrack_info *cm = src;
+	struct xt_conntrack_info m = {
+		.statemask	= cm->statemask,
+		.statusmask	= cm->statusmask,
+		.expires_min	= cm->expires_min,
+		.expires_max	= cm->expires_max,
+		.flags		= cm->flags,
+		.invflags	= cm->invflags,
+	};
+	memcpy(m.tuple, cm->tuple, sizeof(m.tuple));
+	memcpy(m.sipmsk, cm->sipmsk, sizeof(m.sipmsk));
+	memcpy(m.dipmsk, cm->dipmsk, sizeof(m.dipmsk));
+	memcpy(dst, &m, sizeof(m));
+}
+
+static int compat_to_user(void __user *dst, void *src)
+{
+	struct xt_conntrack_info *m = src;
+	struct compat_xt_conntrack_info cm = {
+		.statemask	= m->statemask,
+		.statusmask	= m->statusmask,
+		.expires_min	= m->expires_min,
+		.expires_max	= m->expires_max,
+		.flags		= m->flags,
+		.invflags	= m->invflags,
+	};
+	memcpy(cm.tuple, m->tuple, sizeof(cm.tuple));
+	memcpy(cm.sipmsk, m->sipmsk, sizeof(cm.sipmsk));
+	memcpy(cm.dipmsk, m->dipmsk, sizeof(cm.dipmsk));
+	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
+}
+#endif
+
 static struct xt_match conntrack_match = {
 	.name		= "conntrack",
 	.match		= match,
 	.checkentry	= checkentry,
 	.destroy	= destroy,
 	.matchsize	= sizeof(struct xt_conntrack_info),
+#ifdef CONFIG_COMPAT
+	.compatsize	= sizeof(struct compat_xt_conntrack_info),
+	.compat_from_user = compat_from_user,
+	.compat_to_user	= compat_to_user,
+#endif
 	.family		= AF_INET,
 	.me		= THIS_MODULE,
 };

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Jan Engelhardt wrote:
> Hi,
> 
> 
> the following command gives an error:
> 
> 	iptables -t mangle -I FORWARD -m conntrack --ctstate NEW
> 
> output is:
> 
> 	iptables: Unknown error 4294967295
> 
> As mentioned in the topic, I suspect it is due to 32-bit iptables not 
> coping correctly with the 64-bit kernel (sometimes, patches to fix these 
> are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
> with their latest(?) kernel 2.6.20-1.2986.al3.3smp.


The conntrack match is missing compat support, I've queued this patch
to fix it.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2652 bytes --]

[NETFILTER]: xt_conntrack: add compat support

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit ba8991494e1522be10d764b174fc4e3744c99655
tree 85d5cf3861566aa38ecb2e091be987ecfeb17655
parent 1797736897a68f556aef76a6a0963c3e8b1b4950
author Patrick McHardy <kaber@trash.net> Thu, 10 May 2007 14:57:40 +0200
committer Patrick McHardy <kaber@trash.net> Thu, 10 May 2007 14:57:40 +0200

 net/netfilter/xt_conntrack.c |   54 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index f4ea8fe..189ded5 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -134,12 +134,66 @@ static void destroy(const struct xt_match *match, void *matchinfo)
 	nf_ct_l3proto_module_put(match->family);
 }
 
+#ifdef CONFIG_COMPAT
+struct compat_xt_conntrack_info
+{
+	compat_uint_t			statemask;
+	compat_uint_t			statusmask;
+	struct ip_conntrack_old_tuple	tuple[IP_CT_DIR_MAX];
+	struct in_addr			sipmsk[IP_CT_DIR_MAX];
+	struct in_addr			dipmsk[IP_CT_DIR_MAX];
+	compat_ulong_t			expires_min;
+	compat_ulong_t			expires_max;
+	u_int8_t			flags;
+	u_int8_t			invflags;
+};
+
+static void compat_from_user(void *dst, void *src)
+{
+	struct compat_xt_conntrack_info *cm = src;
+	struct xt_conntrack_info m = {
+		.statemask	= cm->statemask,
+		.statusmask	= cm->statusmask,
+		.expires_min	= cm->expires_min,
+		.expires_max	= cm->expires_max,
+		.flags		= cm->flags,
+		.invflags	= cm->invflags,
+	};
+	memcpy(m.tuple, cm->tuple, sizeof(m.tuple));
+	memcpy(m.sipmsk, cm->sipmsk, sizeof(m.sipmsk));
+	memcpy(m.dipmsk, cm->dipmsk, sizeof(m.dipmsk));
+	memcpy(dst, &m, sizeof(m));
+}
+
+static int compat_to_user(void __user *dst, void *src)
+{
+	struct xt_conntrack_info *m = src;
+	struct compat_xt_conntrack_info cm = {
+		.statemask	= m->statemask,
+		.statusmask	= m->statusmask,
+		.expires_min	= m->expires_min,
+		.expires_max	= m->expires_max,
+		.flags		= m->flags,
+		.invflags	= m->invflags,
+	};
+	memcpy(cm.tuple, m->tuple, sizeof(cm.tuple));
+	memcpy(cm.sipmsk, m->sipmsk, sizeof(cm.sipmsk));
+	memcpy(cm.dipmsk, m->dipmsk, sizeof(cm.dipmsk));
+	return copy_to_user(dst, &cm, sizeof(cm)) ? -EFAULT : 0;
+}
+#endif
+
 static struct xt_match conntrack_match = {
 	.name		= "conntrack",
 	.match		= match,
 	.checkentry	= checkentry,
 	.destroy	= destroy,
 	.matchsize	= sizeof(struct xt_conntrack_info),
+#ifdef CONFIG_COMPAT
+	.compatsize	= sizeof(struct compat_xt_conntrack_info),
+	.compat_from_user = compat_from_user,
+	.compat_to_user	= compat_to_user,
+#endif
 	.family		= AF_INET,
 	.me		= THIS_MODULE,
 };

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 08:27, Jan Engelhardt wrote:
> 
>>As mentioned in the topic, I suspect it is due to 32-bit iptables not 
>>coping correctly with the 64-bit kernel (sometimes, patches to fix these 
>>are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
>>with their latest(?) kernel 2.6.20-1.2986.al3.3smp.
> 
> 
> And the following cmd oopsed it:
> 
> 	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
> 	-j sshcheck;


I believe this is a bug in the compat code, which *seems* to call (its
a bit messy, I just had a quick look) the destroy function without
having called checkentry previously when something goes wrong. Which
commands did you run before this?

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 08:27, Jan Engelhardt wrote:
> 
>>As mentioned in the topic, I suspect it is due to 32-bit iptables not 
>>coping correctly with the 64-bit kernel (sometimes, patches to fix these 
>>are posted, so I thought it could be related). OS is Aurora Linux 2.98, 
>>with their latest(?) kernel 2.6.20-1.2986.al3.3smp.
> 
> 
> And the following cmd oopsed it:
> 
> 	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
> 	-j sshcheck;


I believe this is a bug in the compat code, which *seems* to call (its
a bit messy, I just had a quick look) the destroy function without
having called checkentry previously when something goes wrong. Which
commands did you run before this?

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 15:20, Patrick McHardy wrote:
>> 
>> And the following cmd oopsed it:
>> 
>> 	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
>> 	-j sshcheck;
>
>
>I believe this is a bug in the compat code, which *seems* to call (its
>a bit messy, I just had a quick look) the destroy function without
>having called checkentry previously when something goes wrong. Which
>commands did you run before this?

A lot ... as far as the filter table and sshcheck is concerned,

iptables -N sshcheck;
iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
        --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
        -j RETURN;
iptables -A sshcheck -m recent --name sshcheck --set -j DROP;


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 15:20, Patrick McHardy wrote:
>> 
>> And the following cmd oopsed it:
>> 
>> 	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
>> 	-j sshcheck;
>
>
>I believe this is a bug in the compat code, which *seems* to call (its
>a bit messy, I just had a quick look) the destroy function without
>having called checkentry previously when something goes wrong. Which
>commands did you run before this?

A lot ... as far as the filter table and sshcheck is concerned,

iptables -N sshcheck;
iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
        --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
        -j RETURN;
iptables -A sshcheck -m recent --name sshcheck --set -j DROP;


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 15:20, Patrick McHardy wrote:
> 
>>>And the following cmd oopsed it:
>>>
>>>	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
>>>	-j sshcheck;
>>
>>
>>I believe this is a bug in the compat code, which *seems* to call (its
>>a bit messy, I just had a quick look) the destroy function without
>>having called checkentry previously when something goes wrong. Which
>>commands did you run before this?
> 
> 
> A lot ... as far as the filter table and sshcheck is concerned,
> 
> iptables -N sshcheck;
> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>         -j RETURN;
> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;


Did you get an "invalid size" message in the ringbuffer before the oops?

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 15:20, Patrick McHardy wrote:
> 
>>>And the following cmd oopsed it:
>>>
>>>	# iptables -A INPUT -p tcp --dport 22 -m conntrack --ctstate NEW
>>>	-j sshcheck;
>>
>>
>>I believe this is a bug in the compat code, which *seems* to call (its
>>a bit messy, I just had a quick look) the destroy function without
>>having called checkentry previously when something goes wrong. Which
>>commands did you run before this?
> 
> 
> A lot ... as far as the filter table and sshcheck is concerned,
> 
> iptables -N sshcheck;
> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>         -j RETURN;
> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;


Did you get an "invalid size" message in the ringbuffer before the oops?

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 16:02, Patrick McHardy wrote:
>> 
>> A lot ... as far as the filter table and sshcheck is concerned,
>> 
>> iptables -N sshcheck;
>> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>         -j RETURN;
>> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>
>Did you get an "invalid size" message in the ringbuffer before the oops?

Now that you mention it, yes:

ip_tables: conntrack match: invalid size 80 != 72


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 16:02, Patrick McHardy wrote:
>> 
>> A lot ... as far as the filter table and sshcheck is concerned,
>> 
>> iptables -N sshcheck;
>> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>         -j RETURN;
>> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>
>Did you get an "invalid size" message in the ringbuffer before the oops?

Now that you mention it, yes:

ip_tables: conntrack match: invalid size 80 != 72


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 16:05, Jan Engelhardt wrote:
>On May 10 2007 16:02, Patrick McHardy wrote:
>>> 
>>> A lot ... as far as the filter table and sshcheck is concerned,
>>> 
>>> iptables -N sshcheck;
>>> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>>> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>>         -j RETURN;
>>> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>>
>>Did you get an "invalid size" message in the ringbuffer before the oops?
>
>Now that you mention it, yes:
>
>ip_tables: conntrack match: invalid size 80 != 72

This is fixed in 2.6.21, thanks.


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On May 10 2007 16:05, Jan Engelhardt wrote:
>On May 10 2007 16:02, Patrick McHardy wrote:
>>> 
>>> A lot ... as far as the filter table and sshcheck is concerned,
>>> 
>>> iptables -N sshcheck;
>>> iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>>> iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>>         --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>>         -j RETURN;
>>> iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>>
>>Did you get an "invalid size" message in the ringbuffer before the oops?
>
>Now that you mention it, yes:
>
>ip_tables: conntrack match: invalid size 80 != 72

This is fixed in 2.6.21, thanks.


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 16:05, Jan Engelhardt wrote:
> 
>>On May 10 2007 16:02, Patrick McHardy wrote:
>>
>>>>A lot ... as far as the filter table and sshcheck is concerned,
>>>>
>>>>iptables -N sshcheck;
>>>>iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>>>>iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>>>        --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>>>        -j RETURN;
>>>>iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>>>
>>>Did you get an "invalid size" message in the ringbuffer before the oops?
>>
>>Now that you mention it, yes:
>>
>>ip_tables: conntrack match: invalid size 80 != 72
> 
> 
> This is fixed in 2.6.21, thanks.


Yes, the hashlimit compat issue is. But the underlying problem still
persists, I'll send you a patch for testing soon.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On May 10 2007 16:05, Jan Engelhardt wrote:
> 
>>On May 10 2007 16:02, Patrick McHardy wrote:
>>
>>>>A lot ... as far as the filter table and sshcheck is concerned,
>>>>
>>>>iptables -N sshcheck;
>>>>iptables -A sshcheck -m recent --name sshcheck --seconds 60 --update -j DROP;
>>>>iptables -A sshcheck -m hashlimit --hashlimit-name sshcheck \
>>>>        --hashlimit-mode srcip --hashlimit 4/min --hashlimit-burst 4 \
>>>>        -j RETURN;
>>>>iptables -A sshcheck -m recent --name sshcheck --set -j DROP;
>>>
>>>Did you get an "invalid size" message in the ringbuffer before the oops?
>>
>>Now that you mention it, yes:
>>
>>ip_tables: conntrack match: invalid size 80 != 72
> 
> 
> This is fixed in 2.6.21, thanks.


Yes, the hashlimit compat issue is. But the underlying problem still
persists, I'll send you a patch for testing soon.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Patrick McHardy wrote:
> Jan Engelhardt wrote:
> 
>>This is fixed in 2.6.21, thanks.
> 
> 
> 
> Yes, the hashlimit compat issue is. But the underlying problem still
> persists, I'll send you a patch for testing soon.


Here it is, could you please test whether it fixes the crash by
backing out the hashlimit compat patch and triggering the size
error again? Thanks.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3035 bytes --]

[NETFILTER]: ip_tables: fix compat related crash

check_compat_entry_size_and_hooks iterates over the matches and calls
compat_check_calc_match, which loads the match and calculates the
compat offsets, but unlike the non-compat version, doesn't call
->checkentry yet. On error however it calls cleanup_matches, which in
turn calls ->destroy, which can result in crashes if the destroy
function (validly) expects to only get called after the checkentry
function.

Add a compat_release_match function that only drops the module reference
on error and rename compat_check_calc_match to compat_find_calc_match to
reflect the fact that it doesn't call the checkentry function.

Reported by Jan Engelhardt <jengelh@linux01.gwdg.de>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit aea9d0eb2c80de9d31a9ecbcaf5e529b7503ea13
tree f014a00779dc3c3745e3294a393941ab91c3d37e
parent e7d815ef75f70dcdf55001f1f88ae7ae8827a7ba
author Patrick McHardy <kaber@trash.net> Sat, 02 Jun 2007 14:51:13 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 02 Jun 2007 14:51:13 +0200

 net/ipv4/netfilter/ip_tables.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..8fde1d1 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1425,7 +1425,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1449,16 @@ compat_check_calc_match(struct ipt_entry_match *m,
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- == 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1495,10 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1509,7 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1536,8 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Patrick McHardy wrote:
> Jan Engelhardt wrote:
> 
>>This is fixed in 2.6.21, thanks.
> 
> 
> 
> Yes, the hashlimit compat issue is. But the underlying problem still
> persists, I'll send you a patch for testing soon.


Here it is, could you please test whether it fixes the crash by
backing out the hashlimit compat patch and triggering the size
error again? Thanks.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3035 bytes --]

[NETFILTER]: ip_tables: fix compat related crash

check_compat_entry_size_and_hooks iterates over the matches and calls
compat_check_calc_match, which loads the match and calculates the
compat offsets, but unlike the non-compat version, doesn't call
->checkentry yet. On error however it calls cleanup_matches, which in
turn calls ->destroy, which can result in crashes if the destroy
function (validly) expects to only get called after the checkentry
function.

Add a compat_release_match function that only drops the module reference
on error and rename compat_check_calc_match to compat_find_calc_match to
reflect the fact that it doesn't call the checkentry function.

Reported by Jan Engelhardt <jengelh@linux01.gwdg.de>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit aea9d0eb2c80de9d31a9ecbcaf5e529b7503ea13
tree f014a00779dc3c3745e3294a393941ab91c3d37e
parent e7d815ef75f70dcdf55001f1f88ae7ae8827a7ba
author Patrick McHardy <kaber@trash.net> Sat, 02 Jun 2007 14:51:13 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 02 Jun 2007 14:51:13 +0200

 net/ipv4/netfilter/ip_tables.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..8fde1d1 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1425,7 +1425,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1449,16 @@ compat_check_calc_match(struct ipt_entry_match *m,
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- == 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1495,10 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1509,7 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1536,8 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 2 2007 14:54, Patrick McHardy wrote:
>>>This is fixed in 2.6.21, thanks.
>> 
>> Yes, the hashlimit compat issue is. But the underlying problem still
>> persists, I'll send you a patch for testing soon.
>
>Here it is, could you please test whether it fixes the crash by
>backing out the hashlimit compat patch and triggering the size
>error again? Thanks.

Do you mean conntrack compat?
http://lists.netfilter.org/pipermail/netfilter-devel/2007-May/027763.html


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 2 2007 14:54, Patrick McHardy wrote:
>>>This is fixed in 2.6.21, thanks.
>> 
>> Yes, the hashlimit compat issue is. But the underlying problem still
>> persists, I'll send you a patch for testing soon.
>
>Here it is, could you please test whether it fixes the crash by
>backing out the hashlimit compat patch and triggering the size
>error again? Thanks.

Do you mean conntrack compat?
http://lists.netfilter.org/pipermail/netfilter-devel/2007-May/027763.html


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 2 2007 14:54, Patrick McHardy wrote:
> 
>>>>This is fixed in 2.6.21, thanks.
>>>
>>>Yes, the hashlimit compat issue is. But the underlying problem still
>>>persists, I'll send you a patch for testing soon.
>>
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> 
> Do you mean conntrack compat?
> http://lists.netfilter.org/pipermail/netfilter-devel/2007-May/027763.html


Right, I confused them.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 2 2007 14:54, Patrick McHardy wrote:
> 
>>>>This is fixed in 2.6.21, thanks.
>>>
>>>Yes, the hashlimit compat issue is. But the underlying problem still
>>>persists, I'll send you a patch for testing soon.
>>
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> 
> Do you mean conntrack compat?
> http://lists.netfilter.org/pipermail/netfilter-devel/2007-May/027763.html


Right, I confused them.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 2 2007 14:54, Patrick McHardy wrote:
>Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>> 
>>>This is fixed in 2.6.21, thanks.
>> 
>> Yes, the hashlimit compat issue is. But the underlying problem still
>> persists, I'll send you a patch for testing soon.
>
>Here it is, could you please test whether it fixes the crash by
>backing out the hashlimit compat patch and triggering the size
>error again? Thanks.
[connlimit compat]

Kernel built, rebooted, no panics or thelike.
So to sum it up, hashlimit works, connlimit works. Seems all nice.


Thanks,
	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 2 2007 14:54, Patrick McHardy wrote:
>Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>> 
>>>This is fixed in 2.6.21, thanks.
>> 
>> Yes, the hashlimit compat issue is. But the underlying problem still
>> persists, I'll send you a patch for testing soon.
>
>Here it is, could you please test whether it fixes the crash by
>backing out the hashlimit compat patch and triggering the size
>error again? Thanks.
[connlimit compat]

Kernel built, rebooted, no panics or thelike.
So to sum it up, hashlimit works, connlimit works. Seems all nice.


Thanks,
	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 2 2007 14:54, Patrick McHardy wrote:
> 
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> [connlimit compat]
> 
> Kernel built, rebooted, no panics or thelike.
> So to sum it up, hashlimit works, connlimit works. Seems all nice.


Thanks Jan. Will send upstream and to -stable soon.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 2 2007 14:54, Patrick McHardy wrote:
> 
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> [connlimit compat]
> 
> Kernel built, rebooted, no panics or thelike.
> So to sum it up, hashlimit works, connlimit works. Seems all nice.


Thanks Jan. Will send upstream and to -stable soon.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Jan Engelhardt wrote:
> >>This is fixed in 2.6.21, thanks.
> >
> > Yes, the hashlimit compat issue is. But the underlying problem still
> > persists, I'll send you a patch for testing soon.
>
> Here it is, could you please test whether it fixes the crash by
> backing out the hashlimit compat patch and triggering the size
> error again? Thanks.
Patrick, 
it looks like translate_compat_table() should be fixed also.

-- 
Thanks,
Dmitry.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Jan Engelhardt wrote:
> >>This is fixed in 2.6.21, thanks.
> >
> > Yes, the hashlimit compat issue is. But the underlying problem still
> > persists, I'll send you a patch for testing soon.
>
> Here it is, could you please test whether it fixes the crash by
> backing out the hashlimit compat patch and triggering the size
> error again? Thanks.
Patrick, 
it looks like translate_compat_table() should be fixed also.

-- 
Thanks,
Dmitry.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Dmitry Mishin wrote:
> On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> 
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> Patrick, 
> it looks like translate_compat_table() should be fixed also.


You're right, thanks for catching this. This patch should be
better.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3737 bytes --]

[NETFILTER]: ip_tables: fix compat related crash

check_compat_entry_size_and_hooks iterates over the matches and calls
compat_check_calc_match, which loads the match and calculates the
compat offsets, but unlike the non-compat version, doesn't call
->checkentry yet. On error however it calls cleanup_matches, which in
turn calls ->destroy, which can result in crashes if the destroy
function (validly) expects to only get called after the checkentry
function.

Add a compat_release_match function that only drops the module reference
on error and rename compat_check_calc_match to compat_find_calc_match to
reflect the fact that it doesn't call the checkentry function.

Reported by Jan Engelhardt <jengelh@linux01.gwdg.de>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit b2b15a77343e2baadee22a5e64f691732874b10b
tree 7d82cf0a9fd3cf77933205a7a1834e958293e293
parent e7d815ef75f70dcdf55001f1f88ae7ae8827a7ba
author Patrick McHardy <kaber@trash.net> Sun, 03 Jun 2007 18:57:04 +0200
committer Patrick McHardy <kaber@trash.net> Sun, 03 Jun 2007 18:57:04 +0200

 net/ipv4/netfilter/ip_tables.c |   41 ++++++++++++++++++++++++++++++++--------
 1 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..0a35639 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1425,7 +1425,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1449,31 @@ compat_check_calc_match(struct ipt_entry_match *m,
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- == 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
+compat_release_entry(struct ipt_entry *e, unsigned int *i)
+{
+	struct ipt_entry_target *t;
+
+	if (i && (*i)-- == 0)
+		return 1;
+
+	/* Cleanup all matches */
+	IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+	t = ipt_get_target(e);
+	module_put(t->u.kernel.target->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1510,10 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1524,7 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1551,8 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 
@@ -1690,13 +1715,13 @@ translate_compat_table(const char *name,
 
 free_newinfo:
 	xt_free_table_info(newinfo);
-out:
 	IPT_ENTRY_ITERATE(entry0, total_size, cleanup_entry, &j);
 	return ret;
 out_unlock:
 	compat_flush_offsets();
 	xt_compat_unlock(AF_INET);
-	goto out;
+	IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+	return ret;
 }
 
 static int

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Dmitry Mishin wrote:
> On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> 
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> Patrick, 
> it looks like translate_compat_table() should be fixed also.


You're right, thanks for catching this. This patch should be
better.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3737 bytes --]

[NETFILTER]: ip_tables: fix compat related crash

check_compat_entry_size_and_hooks iterates over the matches and calls
compat_check_calc_match, which loads the match and calculates the
compat offsets, but unlike the non-compat version, doesn't call
->checkentry yet. On error however it calls cleanup_matches, which in
turn calls ->destroy, which can result in crashes if the destroy
function (validly) expects to only get called after the checkentry
function.

Add a compat_release_match function that only drops the module reference
on error and rename compat_check_calc_match to compat_find_calc_match to
reflect the fact that it doesn't call the checkentry function.

Reported by Jan Engelhardt <jengelh@linux01.gwdg.de>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit b2b15a77343e2baadee22a5e64f691732874b10b
tree 7d82cf0a9fd3cf77933205a7a1834e958293e293
parent e7d815ef75f70dcdf55001f1f88ae7ae8827a7ba
author Patrick McHardy <kaber@trash.net> Sun, 03 Jun 2007 18:57:04 +0200
committer Patrick McHardy <kaber@trash.net> Sun, 03 Jun 2007 18:57:04 +0200

 net/ipv4/netfilter/ip_tables.c |   41 ++++++++++++++++++++++++++++++++--------
 1 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..0a35639 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1425,7 +1425,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1449,31 @@ compat_check_calc_match(struct ipt_entry_match *m,
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- == 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
+compat_release_entry(struct ipt_entry *e, unsigned int *i)
+{
+	struct ipt_entry_target *t;
+
+	if (i && (*i)-- == 0)
+		return 1;
+
+	/* Cleanup all matches */
+	IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+	t = ipt_get_target(e);
+	module_put(t->u.kernel.target->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1510,10 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1524,7 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1551,8 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 
@@ -1690,13 +1715,13 @@ translate_compat_table(const char *name,
 
 free_newinfo:
 	xt_free_table_info(newinfo);
-out:
 	IPT_ENTRY_ITERATE(entry0, total_size, cleanup_entry, &j);
 	return ret;
 out_unlock:
 	compat_flush_offsets();
 	xt_compat_unlock(AF_INET);
-	goto out;
+	IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+	return ret;
 }
 
 static int

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 3 2007 18:57, Patrick McHardy wrote:
>Dmitry Mishin wrote:
>> On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
>> 
>>>Here it is, could you please test whether it fixes the crash by
>>>backing out the hashlimit compat patch and triggering the size
>>>error again? Thanks.
>> 
>> Patrick, 
>> it looks like translate_compat_table() should be fixed also.
>
>You're right, thanks for catching this. This patch should be
>better.

I am kind of impartial when it comes to testing patches, but when
I look through and want to reply to them, doing them inline is
preferred.

Tested-By: Jan Engelhardt <jengelh@gmx.de>



	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 3 2007 18:57, Patrick McHardy wrote:
>Dmitry Mishin wrote:
>> On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
>> 
>>>Here it is, could you please test whether it fixes the crash by
>>>backing out the hashlimit compat patch and triggering the size
>>>error again? Thanks.
>> 
>> Patrick, 
>> it looks like translate_compat_table() should be fixed also.
>
>You're right, thanks for catching this. This patch should be
>better.

I am kind of impartial when it comes to testing patches, but when
I look through and want to reply to them, doing them inline is
preferred.

Tested-By: Jan Engelhardt <jengelh@gmx.de>



	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> I am kind of impartial when it comes to testing patches, but when
> I look through and want to reply to them, doing them inline is
> preferred.

Content-Disposition: inline;
 filename="x"

Works fine in any mailer I've tried.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Jan Engelhardt wrote:
> I am kind of impartial when it comes to testing patches, but when
> I look through and want to reply to them, doing them inline is
> preferred.

Content-Disposition: inline;
 filename="x"

Works fine in any mailer I've tried.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 3 2007 19:31, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> I am kind of impartial when it comes to testing patches, but when
>> I look through and want to reply to them, doing them inline is
>> preferred.
>
>Content-Disposition: inline;
> filename="x"
>
>Works fine in any mailer I've tried.

Try pine.
* Displayed the attachment inline. (I think this needs a config
  option, which is enabled for me).
* On reply, it strips the attachments.
* On export, the attachment is retained.
Time to figure.


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Jan Engelhardt]

On Jun 3 2007 19:31, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> I am kind of impartial when it comes to testing patches, but when
>> I look through and want to reply to them, doing them inline is
>> preferred.
>
>Content-Disposition: inline;
> filename="x"
>
>Works fine in any mailer I've tried.

Try pine.
* Displayed the attachment inline. (I think this needs a config
  option, which is enabled for me).
* On reply, it strips the attachments.
* On export, the attachment is retained.
Time to figure.


	Jan
-- 

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Sunday 03 June 2007 20:57, Patrick McHardy wrote:
> Dmitry Mishin wrote:
> > On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> >>Here it is, could you please test whether it fixes the crash by
> >>backing out the hashlimit compat patch and triggering the size
> >>error again? Thanks.
> >
> > Patrick,
> > it looks like translate_compat_table() should be fixed also.
>
> You're right, thanks for catching this. This patch should be
> better.
It's better, but I see the issue with iterate with compat_check_entry() calls.
If it fails, some of target/matches' check_* functions are called, some not.
Please, review my version of this patch.

Signed-off-by: Dmitry Mishin <dim@openvz.org>
---
diff --git a/include/linux/netfilter_ipv4/ip_tables.h 
b/include/linux/netfilter_ipv4/ip_tables.h
index 2f46dd7..9c294a5 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -264,6 +264,23 @@ ({								\
 	__ret;							\
 })
 
+/* fn returns 0 to continue iteration */
+#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, i, fn, args...) \
+({								\
+	unsigned int __i;					\
+	int __ret = 0;						\
+	struct ipt_entry *__entry;				\
+								\
+	for (__i = i; __i < (size); __i += __entry->next_offset) { \
+		__entry = (void *)(entries) + __i;		\
+								\
+		__ret = fn(__entry , ## args);			\
+		if (__ret != 0)					\
+			break;					\
+	}							\
+	__ret;							\
+})
+
 /*
  *	Main firewall chains definitions and global var's definitions.
  */
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..9bacf1a 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -499,7 +499,8 @@ check_entry(struct ipt_entry *e, const c
 }
 
 static inline int check_match(struct ipt_entry_match *m, const char *name,
-				const struct ipt_ip *ip, unsigned int hookmask)
+				const struct ipt_ip *ip, unsigned int hookmask,
+				unsigned int *i)
 {
 	struct xt_match *match;
 	int ret;
@@ -515,6 +516,8 @@ static inline int check_match(struct ipt
 			 m->u.kernel.match->name);
 		ret = -EINVAL;
 	}
+	if (!ret)
+		(*i)++;
 	return ret;
 }
 
@@ -537,11 +540,10 @@ find_check_match(struct ipt_entry_match 
 	}
 	m->u.kernel.match = match;
 
-	ret = check_match(m, name, ip, hookmask);
+	ret = check_match(m, name, ip, hookmask, i);
 	if (ret)
 		goto err;
 
-	(*i)++;
 	return 0;
 err:
 	module_put(m->u.kernel.match->me);
@@ -1425,7 +1427,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1451,31 @@ compat_check_calc_match(struct ipt_entry
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- == 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
+compat_release_entry(struct ipt_entry *e, unsigned int *i)
+{
+	struct ipt_entry_target *t;
+
+	if (i && (*i)-- == 0)
+		return 1;
+
+	/* Cleanup all matches */
+	IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+	t = ipt_get_target(e);
+	module_put(t->u.kernel.target->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1512,10 @@ check_compat_entry_size_and_hooks(struct
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1526,7 @@ check_compat_entry_size_and_hooks(struct
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1553,8 @@ check_compat_entry_size_and_hooks(struct
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 
@@ -1574,15 +1601,26 @@ static int compat_copy_entry_from_user(s
 	return ret;
 }
 
-static inline int compat_check_entry(struct ipt_entry *e, const char *name)
+static inline int compat_check_entry(struct ipt_entry *e, const char *name,
+						unsigned int *i)
 {
-	int ret;
+	int j, ret;
 
-	ret = IPT_MATCH_ITERATE(e, check_match, name, &e->ip, e->comefrom);
+	j = 0;
+	ret = IPT_MATCH_ITERATE(e, check_match, name, &e->ip, e->comefrom, &j);
 	if (ret)
-		return ret;
+		goto cleanup_matches;
+
+	ret = check_target(e, name);
+	if (ret)
+		goto cleanup_matches;
 
-	return check_target(e, name);
+	(*i)++;
+	return 0;
+
+ cleanup_matches:
+	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+	return ret;
 }
 
 static int
@@ -1673,10 +1711,17 @@ translate_compat_table(const char *name,
 	if (!mark_source_chains(newinfo, valid_hooks, entry1))
 		goto free_newinfo;
 
+	i = 0;
 	ret = IPT_ENTRY_ITERATE(entry1, newinfo->size, compat_check_entry,
-									name);
-	if (ret)
-		goto free_newinfo;
+								name, &i);
+	if (ret) {
+		j -= i;
+		IPT_ENTRY_ITERATE_CONTINUE(entry1, newinfo->size, i,
+						compat_release_entry, &j);
+		IPT_ENTRY_ITERATE(entry1, newinfo->size, cleanup_entry, &i);
+		xt_free_table_info(newinfo);
+		return ret;
+	}
 
 	/* And one copy for every other CPU */
 	for_each_possible_cpu(i)
@@ -1691,7 +1736,7 @@ translate_compat_table(const char *name,
 free_newinfo:
 	xt_free_table_info(newinfo);
 out:
-	IPT_ENTRY_ITERATE(entry0, total_size, cleanup_entry, &j);
+	IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
 	return ret;
 out_unlock:
 	compat_flush_offsets();

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Sunday 03 June 2007 20:57, Patrick McHardy wrote:
> Dmitry Mishin wrote:
> > On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> >>Here it is, could you please test whether it fixes the crash by
> >>backing out the hashlimit compat patch and triggering the size
> >>error again? Thanks.
> >
> > Patrick,
> > it looks like translate_compat_table() should be fixed also.
>
> You're right, thanks for catching this. This patch should be
> better.
It's better, but I see the issue with iterate with compat_check_entry() calls.
If it fails, some of target/matches' check_* functions are called, some not.
Please, review my version of this patch.

Signed-off-by: Dmitry Mishin <dim@openvz.org>
---
diff --git a/include/linux/netfilter_ipv4/ip_tables.h 
b/include/linux/netfilter_ipv4/ip_tables.h
index 2f46dd7..9c294a5 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -264,6 +264,23 @@ ({								\
 	__ret;							\
 })
 
+/* fn returns 0 to continue iteration */
+#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, i, fn, args...) \
+({								\
+	unsigned int __i;					\
+	int __ret = 0;						\
+	struct ipt_entry *__entry;				\
+								\
+	for (__i = i; __i < (size); __i += __entry->next_offset) { \
+		__entry = (void *)(entries) + __i;		\
+								\
+		__ret = fn(__entry , ## args);			\
+		if (__ret != 0)					\
+			break;					\
+	}							\
+	__ret;							\
+})
+
 /*
  *	Main firewall chains definitions and global var's definitions.
  */
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..9bacf1a 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -499,7 +499,8 @@ check_entry(struct ipt_entry *e, const c
 }
 
 static inline int check_match(struct ipt_entry_match *m, const char *name,
-				const struct ipt_ip *ip, unsigned int hookmask)
+				const struct ipt_ip *ip, unsigned int hookmask,
+				unsigned int *i)
 {
 	struct xt_match *match;
 	int ret;
@@ -515,6 +516,8 @@ static inline int check_match(struct ipt
 			 m->u.kernel.match->name);
 		ret = -EINVAL;
 	}
+	if (!ret)
+		(*i)++;
 	return ret;
 }
 
@@ -537,11 +540,10 @@ find_check_match(struct ipt_entry_match 
 	}
 	m->u.kernel.match = match;
 
-	ret = check_match(m, name, ip, hookmask);
+	ret = check_match(m, name, ip, hookmask, i);
 	if (ret)
 		goto err;
 
-	(*i)++;
 	return 0;
 err:
 	module_put(m->u.kernel.match->me);
@@ -1425,7 +1427,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
 	    const char *name,
 	    const struct ipt_ip *ip,
 	    unsigned int hookmask,
@@ -1449,6 +1451,31 @@ compat_check_calc_match(struct ipt_entry
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+	if (i && (*i)-- = 0)
+		return 1;
+
+	module_put(m->u.kernel.match->me);
+	return 0;
+}
+
+static inline int
+compat_release_entry(struct ipt_entry *e, unsigned int *i)
+{
+	struct ipt_entry_target *t;
+
+	if (i && (*i)-- = 0)
+		return 1;
+
+	/* Cleanup all matches */
+	IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+	t = ipt_get_target(e);
+	module_put(t->u.kernel.target->me);
+	return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
 			   struct xt_table_info *newinfo,
 			   unsigned int *size,
@@ -1485,10 +1512,10 @@ check_compat_entry_size_and_hooks(struct
 	off = 0;
 	entry_offset = (void *)e - (void *)base;
 	j = 0;
-	ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+	ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
 			e->comefrom, &off, &j);
 	if (ret != 0)
-		goto cleanup_matches;
+		goto release_matches;
 
 	t = ipt_get_target(e);
 	target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1526,7 @@ check_compat_entry_size_and_hooks(struct
 		duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
 							t->u.user.name);
 		ret = target ? PTR_ERR(target) : -ENOENT;
-		goto cleanup_matches;
+		goto release_matches;
 	}
 	t->u.kernel.target = target;
 
@@ -1526,8 +1553,8 @@ check_compat_entry_size_and_hooks(struct
 
 out:
 	module_put(t->u.kernel.target->me);
-cleanup_matches:
-	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+	IPT_MATCH_ITERATE(e, compat_release_match, &j);
 	return ret;
 }
 
@@ -1574,15 +1601,26 @@ static int compat_copy_entry_from_user(s
 	return ret;
 }
 
-static inline int compat_check_entry(struct ipt_entry *e, const char *name)
+static inline int compat_check_entry(struct ipt_entry *e, const char *name,
+						unsigned int *i)
 {
-	int ret;
+	int j, ret;
 
-	ret = IPT_MATCH_ITERATE(e, check_match, name, &e->ip, e->comefrom);
+	j = 0;
+	ret = IPT_MATCH_ITERATE(e, check_match, name, &e->ip, e->comefrom, &j);
 	if (ret)
-		return ret;
+		goto cleanup_matches;
+
+	ret = check_target(e, name);
+	if (ret)
+		goto cleanup_matches;
 
-	return check_target(e, name);
+	(*i)++;
+	return 0;
+
+ cleanup_matches:
+	IPT_MATCH_ITERATE(e, cleanup_match, &j);
+	return ret;
 }
 
 static int
@@ -1673,10 +1711,17 @@ translate_compat_table(const char *name,
 	if (!mark_source_chains(newinfo, valid_hooks, entry1))
 		goto free_newinfo;
 
+	i = 0;
 	ret = IPT_ENTRY_ITERATE(entry1, newinfo->size, compat_check_entry,
-									name);
-	if (ret)
-		goto free_newinfo;
+								name, &i);
+	if (ret) {
+		j -= i;
+		IPT_ENTRY_ITERATE_CONTINUE(entry1, newinfo->size, i,
+						compat_release_entry, &j);
+		IPT_ENTRY_ITERATE(entry1, newinfo->size, cleanup_entry, &i);
+		xt_free_table_info(newinfo);
+		return ret;
+	}
 
 	/* And one copy for every other CPU */
 	for_each_possible_cpu(i)
@@ -1691,7 +1736,7 @@ translate_compat_table(const char *name,
 free_newinfo:
 	xt_free_table_info(newinfo);
 out:
-	IPT_ENTRY_ITERATE(entry0, total_size, cleanup_entry, &j);
+	IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
 	return ret;
 out_unlock:
 	compat_flush_offsets();

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Dmitry Mishin wrote:
> It's better, but I see the issue with iterate with compat_check_entry() calls.
> If it fails, some of target/matches' check_* functions are called, some not.
> Please, review my version of this patch.


You're right again, thanks. Patch applied.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

Dmitry Mishin wrote:
> It's better, but I see the issue with iterate with compat_check_entry() calls.
> If it fails, some of target/matches' check_* functions are called, some not.
> Please, review my version of this patch.


You're right again, thanks. Patch applied.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Patrick McHardy wrote:
> Dmitry Mishin wrote:
> 
>>It's better, but I see the issue with iterate with compat_check_entry() calls.
>>If it fails, some of target/matches' check_* functions are called, some not.
>>Please, review my version of this patch.
> 
> 
> 
> You're right again, thanks. Patch applied.
> 

With one minor change:


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 915 bytes --]

diff --git a/include/linux/netfilter_ipv4/ip_tables.h b/include/linux/netfilter_ipv4/ip_tables.h
index 9c294a5..e992cd6 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -265,14 +265,17 @@ ipt_get_target(struct ipt_entry *e)
 })
 
 /* fn returns 0 to continue iteration */
-#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, i, fn, args...) \
+#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, n, fn, args...) \
 ({								\
-	unsigned int __i;					\
+	unsigned int __i, __n;					\
 	int __ret = 0;						\
 	struct ipt_entry *__entry;				\
 								\
-	for (__i = i; __i < (size); __i += __entry->next_offset) { \
+	for (__i = 0, __n = 0; __i < (size);			\
+	     __i += __entry->next_offset, __n++) { 		\
 		__entry = (void *)(entries) + __i;		\
+		if (__n < n)					\
+			continue;				\
 								\
 		__ret = fn(__entry , ## args);			\
 		if (__ret != 0)					\

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Patrick McHardy]

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

Patrick McHardy wrote:
> Dmitry Mishin wrote:
> 
>>It's better, but I see the issue with iterate with compat_check_entry() calls.
>>If it fails, some of target/matches' check_* functions are called, some not.
>>Please, review my version of this patch.
> 
> 
> 
> You're right again, thanks. Patch applied.
> 

With one minor change:


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 915 bytes --]

diff --git a/include/linux/netfilter_ipv4/ip_tables.h b/include/linux/netfilter_ipv4/ip_tables.h
index 9c294a5..e992cd6 100644
--- a/include/linux/netfilter_ipv4/ip_tables.h
+++ b/include/linux/netfilter_ipv4/ip_tables.h
@@ -265,14 +265,17 @@ ipt_get_target(struct ipt_entry *e)
 })
 
 /* fn returns 0 to continue iteration */
-#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, i, fn, args...) \
+#define IPT_ENTRY_ITERATE_CONTINUE(entries, size, n, fn, args...) \
 ({								\
-	unsigned int __i;					\
+	unsigned int __i, __n;					\
 	int __ret = 0;						\
 	struct ipt_entry *__entry;				\
 								\
-	for (__i = i; __i < (size); __i += __entry->next_offset) { \
+	for (__i = 0, __n = 0; __i < (size);			\
+	     __i += __entry->next_offset, __n++) { 		\
 		__entry = (void *)(entries) + __i;		\
+		if (__n < n)					\
+			continue;				\
 								\
 		__ret = fn(__entry , ## args);			\
 		if (__ret != 0)					\

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Tuesday 05 June 2007 17:32, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Dmitry Mishin wrote:
> >>It's better, but I see the issue with iterate with compat_check_entry()
> >> calls. If it fails, some of target/matches' check_* functions are
> >> called, some not. Please, review my version of this patch.
> >
> > You're right again, thanks. Patch applied.
>
> With one minor change:
No probs, thanks Patrick, for your work!

-- 
Thanks,
Dmitry.

Re: iptables throws unknown error - suspecting 32/64 compat issue

[Dmitry Mishin]

On Tuesday 05 June 2007 17:32, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Dmitry Mishin wrote:
> >>It's better, but I see the issue with iterate with compat_check_entry()
> >> calls. If it fails, some of target/matches' check_* functions are
> >> called, some not. Please, review my version of this patch.
> >
> > You're right again, thanks. Patch applied.
>
> With one minor change:
No probs, thanks Patrick, for your work!

-- 
Thanks,
Dmitry.