diff mbox

iptables/ipset "-m set" alignment problem 64bit kernel 32bit userspace

Message ID alpine.DEB.2.11.1410312256090.1586@aurora.sdinet.de
State Not Applicable
Delegated to: Pablo Neira
Headers show

Commit Message

Sven-Haegar Koch Oct. 31, 2014, 10:30 p.m. UTC 
Hallo,

Problem with 32bit userspace iptables, 64bit kernel and the "-m set" 
ipset match.

iptables:
32bit, debian 1.4.21-2 plus the ipset patch from the git branch

kernel:
64bit, debian 3.2.63-2+deb7u1 plus ipset 6.23

When trying to add an iptables set match it fails with the following 
error when using 64bit kernel and 32bit userspace:

sims:~# iptables -A OUTPUT -m set --match-set testset src -j ACCEPT
iptables: Invalid argument. Run `dmesg' for more information.

In syslog:
x_tables: ip_tables: set.3 match: invalid size 48 (kernel) != (user) 32


Adding some hacky paddings to the userspace iptables makes it work with 
my 64bit kernel, but this way is naturally no real solution:

(whitespace damaged, cut&paste)



I do not see a way to cleanly fix the revision 3 set match, as any 
change would break it for either existing 32+32 or 64+64 environments - 
the only clean way I see would be a revision 4 that works correctly and 
accept that rev 3 does not work in the mixed environment.

c'ya
sven-haegar

Comments

Florian Westphal Oct. 31, 2014, 10:50 p.m. UTC | #1 
Sven-Haegar Koch <haegar@sdinet.de> wrote:
> Hallo,
> 
> Problem with 32bit userspace iptables, 64bit kernel and the "-m set" 
> ipset match.
> 
> iptables:
> 32bit, debian 1.4.21-2 plus the ipset patch from the git branch
> 
> kernel:
> 64bit, debian 3.2.63-2+deb7u1 plus ipset 6.23
> 
> When trying to add an iptables set match it fails with the following 
> error when using 64bit kernel and 32bit userspace:
> 
> sims:~# iptables -A OUTPUT -m set --match-set testset src -j ACCEPT
> iptables: Invalid argument. Run `dmesg' for more information.
> 
> In syslog:
> x_tables: ip_tables: set.3 match: invalid size 48 (kernel) != (user) 32
> 
> 
> Adding some hacky paddings to the userspace iptables makes it work with 
> my 64bit kernel, but this way is naturally no real solution:
> 
> (whitespace damaged, cut&paste)
> 
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -238,6 +238,7 @@ enum {
>  
>  struct ip_set_counter_match {
>         __u8 op;
> +       __u8 padding[7];
>         __u64 value;

Ouch.

> I do not see a way to cleanly fix the revision 3 set match, as any 
> change would break it for either existing 32+32 or 64+64 environments -

Right.

The unclean fix is to provide compat fixup hooks to transparently
convert it in the kernel.

See net/netfilter/xt_limit.c for full example, essentially the
target/match description has to provide

static struct xt_match limit_mt_reg __read_mostly = {
        .name             = "limit",
[..]
#ifdef CONFIG_COMPAT
        .compatsize       = sizeof(struct compat_xt_rateinfo),
        .compat_from_user = limit_mt_compat_from_user,
        .compat_to_user   = limit_mt_compat_to_user,
#endif

The size of the 32bit layout and convert hooks that translate
from the 32 to 64 bit layout (and vice versa).

Jozsef -- v4 or compat crap? :-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt Oct. 31, 2014, 10:57 p.m. UTC | #2 
On Friday 2014-10-31 23:50, Florian Westphal wrote:
>
>The unclean fix is to provide compat fixup hooks to transparently
>convert it in the kernel.
>
>[..]
>#ifdef CONFIG_COMPAT
>        .compatsize       = sizeof(struct compat_xt_rateinfo),
>        .compat_from_user = limit_mt_compat_from_user,
>        .compat_to_user   = limit_mt_compat_to_user,
>#endif
>
>The size of the 32bit layout and convert hooks that translate
>from the 32 to 64 bit layout (and vice versa).

That is not going to work, especially if you have more than three or
more ABIs in the system. Like, o32/n32/64 for mips (or
i386/x86_64/x32). A report about that happens about every year on the
mailing list.

The only fix is to define a v4, and to make sure that there is no
implicit padding of 32 bits or more.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jozsef Kadlecsik Nov. 1, 2014, 9:51 a.m. UTC | #3 
On Fri, 31 Oct 2014, Florian Westphal wrote:

> Sven-Haegar Koch <haegar@sdinet.de> wrote:
> > Hallo,
> > 
> > Problem with 32bit userspace iptables, 64bit kernel and the "-m set" 
> > ipset match.
> > 
> > iptables:
> > 32bit, debian 1.4.21-2 plus the ipset patch from the git branch
> > 
> > kernel:
> > 64bit, debian 3.2.63-2+deb7u1 plus ipset 6.23
> > 
> > When trying to add an iptables set match it fails with the following 
> > error when using 64bit kernel and 32bit userspace:
> > 
> > sims:~# iptables -A OUTPUT -m set --match-set testset src -j ACCEPT
> > iptables: Invalid argument. Run `dmesg' for more information.
> > 
> > In syslog:
> > x_tables: ip_tables: set.3 match: invalid size 48 (kernel) != (user) 32
> > 
> > 
> > Adding some hacky paddings to the userspace iptables makes it work with 
> > my 64bit kernel, but this way is naturally no real solution:
> > 
> > (whitespace damaged, cut&paste)
> > 
> > --- a/include/linux/netfilter/ipset/ip_set.h
> > +++ b/include/linux/netfilter/ipset/ip_set.h
> > @@ -238,6 +238,7 @@ enum {
> >  
> >  struct ip_set_counter_match {
> >         __u8 op;
> > +       __u8 padding[7];
> >         __u64 value;
> 
> Ouch.
> 
> > I do not see a way to cleanly fix the revision 3 set match, as any 
> > change would break it for either existing 32+32 or 64+64 environments -
> 
> Right.
> 
> The unclean fix is to provide compat fixup hooks to transparently
> convert it in the kernel.
> 
> See net/netfilter/xt_limit.c for full example, essentially the
> target/match description has to provide
> 
> static struct xt_match limit_mt_reg __read_mostly = {
>         .name             = "limit",
> [..]
> #ifdef CONFIG_COMPAT
>         .compatsize       = sizeof(struct compat_xt_rateinfo),
>         .compat_from_user = limit_mt_compat_from_user,
>         .compat_to_user   = limit_mt_compat_to_user,
> #endif
> 
> The size of the 32bit layout and convert hooks that translate
> from the 32 to 64 bit layout (and vice versa).
> 
> Jozsef -- v4 or compat crap? :-)

v4 of course! That's the best way to go...

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -238,6 +238,7 @@  enum {
 
 struct ip_set_counter_match {
        __u8 op;
+       __u8 padding[7];
        __u64 value;
 };
 
--- a/include/linux/netfilter/xt_set.h
+++ b/include/linux/netfilter/xt_set.h
@@ -66,9 +66,11 @@  struct xt_set_info_target_v2 {
 
 struct xt_set_info_match_v3 {
        struct xt_set_info match_set;
+       __u32 padding1;
        struct ip_set_counter_match packets;
        struct ip_set_counter_match bytes;
        __u32 flags;
+       __u32 padding2;
 };
 
 /* Revision 3 target */