[2.6.24][BUG] Compact code broken?

[2.6.24][BUG] Compact code broken?

[Luca Tettamanti]

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

Hello,
I'm testing a 64bit kernel on my machine and I've found an issue with
iptables (32 bit). Kernel is git current (9418d5dc).

I'm unable to add any rule to the filter table, with kernel returning
EFAULT:

socket(PF_INET, SOCK_RAW, IPPROTO_RAW)  = 3
getsockopt(3, SOL_IP, 0x40 /* IP_??? */, "filter\0\377@\212r\200\377\377\377\3778 \230)\0\201\377"..., [84]) = 0
getsockopt(3, SOL_IP, 0x41 /* IP_??? */, "filter\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., [656]) = 0
setsockopt(3, SOL_IP, 0x40 /* IP_??? */, "filter\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 860) = -1 EFAULT (Bad address)
write(2, "iptables: Bad address\n", 22) = 22

With a bit of printk I've tracked it to the first copy_from_user in
compat_do_replace:

compat_do_replace:1859: user = ffffffff883cc370, len = 860
compat_do_replace:1864: copy_from_user = 92

Userspace reports:

iptc_init: valid_hooks=0x0000000e, num_entries=4, size=620
cache_add_entry: entering...0:0 new builtin chain: 0x8055910 (rules=0x8055960)
0:0 normal rule: 0x8055980: standard, verdict=-2
cache_add_entry: entering...1:148 new builtin chain: 0x8055a40 (rules=0x8055a90)
iptcc_delete_rule: deleting rule 0x8055980 (offset 0)
1:148 normal rule: 0x8055980: standard, verdict=-2
cache_add_entry: entering...2:296 new builtin chain: 0x8055ab0 (rules=0x8055b00)
iptcc_delete_rule: deleting rule 0x8055980 (offset 148)
2:296 normal rule: 0x8055980: standard, verdict=-2
cache_add_entry: entering...3:444: end of table:
iptcc_delete_rule: deleting rule 0x8055980 (offset 296)
iptcc_compile_chain_offsets: INPUT: chain_head 0, offset=0
iptcc_compile_chain_offsets: rule 0, offset=0, index=0
iptcc_compile_chain_offsets: INPUT; chain_foot 1, offset=148, index=1
iptcc_compile_chain_offsets: FORWARD: chain_head 2, offset=296
iptcc_compile_chain_offsets: FORWARD; chain_foot 2, offset=296, index=2
iptcc_compile_chain_offsets: OUTPUT: chain_head 3, offset=444
iptcc_compile_chain_offsets: OUTPUT; chain_foot 3, offset=444, index=3
iptc_commit: num_entries=5, size=768, num_counters=4
iptc_commit: repl=0x8055be0 <--- this is the argument of setsockopt
iptables: Bad address

I'm also attaching the 2 blobs created by iptables.

The test command was:
iptables -A INPUT -s 123.123.123.132 -i eth0 -j DROP

32 bit userspace with 32 kernel works fine; I'm unable to test with 64
bit userspace (it would require a big surgery on my existing 32 bit
installation).

Luca
-- 
"In linea di principio sarei indifferente al natale, se solo il natale
 ricambiasse la cortesia e mi lasciasse in pace." -- Marco d'Itri

[-- Attachment #2: blobs.tar.gz --]
[-- Type: application/octet-stream, Size: 358 bytes --]

Re: [2.6.24][BUG] Compact code broken?

[David Miller]

From: Luca Tettamanti <kronos.it@gmail.com>
Date: Wed, 14 Nov 2007 22:28:55 +0100

> Hello,
> I'm testing a 64bit kernel on my machine and I've found an issue with
> iptables (32 bit). Kernel is git current (9418d5dc).

What platform?  x86?  powerpc?  sparc64?

Re: [2.6.24][BUG] Compact code broken?

[Luca Tettamanti]

On Nov 15, 2007 12:00 AM, David Miller <davem@davemloft.net> wrote:
> From: Luca Tettamanti <kronos.it@gmail.com>
> Date: Wed, 14 Nov 2007 22:28:55 +0100
>
> > Hello,
> > I'm testing a 64bit kernel on my machine and I've found an issue with
> > iptables (32 bit). Kernel is git current (9418d5dc).
>
> What platform?  x86?  powerpc?  sparc64?

x86

Luca

Re: [2.6.24][BUG] Compact code broken?

[David Miller]

From: "Luca Tettamanti" <kronos.it@gmail.com>
Date: Thu, 15 Nov 2007 00:39:11 +0100

> On Nov 15, 2007 12:00 AM, David Miller <davem@davemloft.net> wrote:
> > From: Luca Tettamanti <kronos.it@gmail.com>
> > Date: Wed, 14 Nov 2007 22:28:55 +0100
> >
> > > Hello,
> > > I'm testing a 64bit kernel on my machine and I've found an issue with
> > > iptables (32 bit). Kernel is git current (9418d5dc).
> >
> > What platform?  x86?  powerpc?  sparc64?
> 
> x86

Ok, the userland pointer you provided seemed to be sign extended
to 64-bit, so there might be a pointer arithmetic bug in the
netfilter compat code somewhere.

Re: [2.6.24][BUG] Compact code broken?

[Patrick McHardy]

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

David Miller wrote:
> From: "Luca Tettamanti" <kronos.it@gmail.com>
> Date: Thu, 15 Nov 2007 00:39:11 +0100
> 
>> On Nov 15, 2007 12:00 AM, David Miller <davem@davemloft.net> wrote:
>>> From: Luca Tettamanti <kronos.it@gmail.com>
>>> Date: Wed, 14 Nov 2007 22:28:55 +0100
>>>
>>>> Hello,
>>>> I'm testing a 64bit kernel on my machine and I've found an issue with
>>>> iptables (32 bit). Kernel is git current (9418d5dc).
>>> What platform?  x86?  powerpc?  sparc64?
>> x86
> 
> Ok, the userland pointer you provided seemed to be sign extended
> to 64-bit, so there might be a pointer arithmetic bug in the
> netfilter compat code somewhere.


It took me a few passes over the code, but it turns out to be
a simple typo :)

Luca, does this fix it for you?


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

[NETFILTER]: fix compat_nf_sockopt typo

It should pass opt to the ->get/->set functions, not ops.

Fixes compat fault reported by Luca Tettamanti <kronos.it@gmail.com>

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

---
commit ac55193be35872211189096ec600f94614a0179b
tree a6633e33307b5ec1e11fedc2d0e125ed6e2bd081
parent 99fee6d7e5748d96884667a4628118f7fc130ea0
author Patrick McHardy <kaber@trash.net> Thu, 15 Nov 2007 14:46:43 +0100
committer Patrick McHardy <kaber@trash.net> Thu, 15 Nov 2007 14:46:43 +0100

 net/netfilter/nf_sockopt.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_sockopt.c b/net/netfilter/nf_sockopt.c
index 87bc144..3dd4b3c 100644
--- a/net/netfilter/nf_sockopt.c
+++ b/net/netfilter/nf_sockopt.c
@@ -143,12 +143,12 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 		if (ops->compat_get)
 			ret = ops->compat_get(sk, val, opt, len);
 		else
-			ret = ops->get(sk, val, ops, len);
+			ret = ops->get(sk, val, opt, len);
 	} else {
 		if (ops->compat_set)
-			ret = ops->compat_set(sk, val, ops, *len);
+			ret = ops->compat_set(sk, val, opt, *len);
 		else
-			ret = ops->set(sk, val, ops, *len);
+			ret = ops->set(sk, val, opt, *len);
 	}
 
 	module_put(ops->owner);

Re: [2.6.24][BUG] Compact code broken?

[Luca Tettamanti]

On Nov 15, 2007 2:48 PM, Patrick McHardy <kaber@trash.net> wrote:
> David Miller wrote:
> > From: "Luca Tettamanti" <kronos.it@gmail.com>
> > Date: Thu, 15 Nov 2007 00:39:11 +0100
> >
> >> On Nov 15, 2007 12:00 AM, David Miller <davem@davemloft.net> wrote:
> >>> From: Luca Tettamanti <kronos.it@gmail.com>
> >>> Date: Wed, 14 Nov 2007 22:28:55 +0100
> >>>
> >>>> Hello,
> >>>> I'm testing a 64bit kernel on my machine and I've found an issue with
> >>>> iptables (32 bit). Kernel is git current (9418d5dc).
> >>> What platform?  x86?  powerpc?  sparc64?
> >> x86
> >
> > Ok, the userland pointer you provided seemed to be sign extended
> > to 64-bit, so there might be a pointer arithmetic bug in the
> > netfilter compat code somewhere.
>
>
> It took me a few passes over the code, but it turns out to be
> a simple typo :)
>
> Luca, does this fix it for you?

Yes, it's working fine, thank you!

> [NETFILTER]: fix compat_nf_sockopt typo
>
> It should pass opt to the ->get/->set functions, not ops.
>
> Fixes compat fault reported by Luca Tettamanti <kronos.it@gmail.com>
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Tested-by: Luca Tettamanti <kronos.it@gmail.com>

>
> ---
> commit ac55193be35872211189096ec600f94614a0179b
> tree a6633e33307b5ec1e11fedc2d0e125ed6e2bd081
> parent 99fee6d7e5748d96884667a4628118f7fc130ea0
> author Patrick McHardy <kaber@trash.net> Thu, 15 Nov 2007 14:46:43 +0100
> committer Patrick McHardy <kaber@trash.net> Thu, 15 Nov 2007 14:46:43 +0100
>
>  net/netfilter/nf_sockopt.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/nf_sockopt.c b/net/netfilter/nf_sockopt.c
> index 87bc144..3dd4b3c 100644
> --- a/net/netfilter/nf_sockopt.c
> +++ b/net/netfilter/nf_sockopt.c
> @@ -143,12 +143,12 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
>                 if (ops->compat_get)
>                         ret = ops->compat_get(sk, val, opt, len);
>                 else
> -                       ret = ops->get(sk, val, ops, len);
> +                       ret = ops->get(sk, val, opt, len);
>         } else {
>                 if (ops->compat_set)
> -                       ret = ops->compat_set(sk, val, ops, *len);
> +                       ret = ops->compat_set(sk, val, opt, *len);
>                 else
> -                       ret = ops->set(sk, val, ops, *len);
> +                       ret = ops->set(sk, val, opt, *len);
>         }
>
>         module_put(ops->owner);
>
>