Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1407494

Re: [Regression?] iptables broken on 32bit with pre-4.7-rc

From Florian Westphal <fw@strlen.de>
Newsgroups linux.kernel
Subject Re: [Regression?] iptables broken on 32bit with pre-4.7-rc
Date 2016-05-26 12:00 +0200
Message-ID <rD0f8-4Ml-23@gated-at.bofh.it> (permalink)
References <rCWuS-2rd-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


John Stultz <john.stultz@linaro.org> wrote:
> In updating a 32bit arm device from 4.6 to Linus' current HEAD, I
> noticed I was having some trouble with networking, and realized that
> /proc/net/ip_tables_names was suddenly empty.
> 
> Digging through the registration process, it seems we're catching on the:
> 
>        if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
>            target_offset + sizeof(struct xt_standard_target) != next_offset)
>                return -EINVAL;
> 
> check added in 7ed2abddd20cf ("netfilter: x_tables: check standard
> target size too").
> 
> Where next_offset seems to be 4 bytes larger then the the offset +
> standard_target struct size.

I guess its because arm32 needs 8 byte alignment for 64bit
quantities.  So we can fix this either via XT_ALIGN()'ing the
target_offset + sizeof() result or by weakening the test to a '>'.

Since we already test proper alignment of start-of-rule in
check_entry_size_and_hooks() I'd suggest we just change the test
to fail only if the next offset is within the min size, i.e.:

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index c69c892..9643047 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -612,7 +612,7 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,
                return -EINVAL;
 
        if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           target_offset + sizeof(struct compat_xt_standard_target) != next_offset)
+           target_offset + sizeof(struct compat_xt_standard_target) > next_offset)
                return -EINVAL;
 
        /* compat_xt_entry match has less strict aligment requirements,
@@ -694,7 +694,7 @@ int xt_check_entry_offsets(const void *base,
                return -EINVAL;
 
        if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           target_offset + sizeof(struct xt_standard_target) != next_offset)
+           target_offset + sizeof(struct xt_standard_target) > next_offset)
                return -EINVAL;
 
        return xt_check_entry_match(elems, base + target_offset,

> I'm not exactly sure how the next_offset value is set, so I'm hoping
> the proper fix is more obvious to one of you.

Its the start of the next rule so it has to be properly aligned
via XT_ALIGN().  Only 32bit system I tested was plain x86 which
only needs 4byte alignment for u64...

Alternative would be something like this:

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index c69c892..ca16c26 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -612,7 +612,7 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,
                return -EINVAL;
 
        if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           target_offset + sizeof(struct compat_xt_standard_target) != next_offset)
+           XT_COMPAT_ALIGN(target_offset + sizeof(struct compat_xt_standard_target)) != next_offset)
                return -EINVAL;
 
        /* compat_xt_entry match has less strict aligment requirements,
@@ -694,7 +694,7 @@ int xt_check_entry_offsets(const void *base,
                return -EINVAL;
 
        if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-           target_offset + sizeof(struct xt_standard_target) != next_offset)
+           XT_ALIGN(target_offset + sizeof(struct xt_standard_target)) != next_offset)
                return -EINVAL;
 
        return xt_check_entry_match(elems, base + target_offset,


but afaics the stricter check does not buy anything.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[Regression?] iptables broken on 32bit with pre-4.7-rc John Stultz <john.stultz@linaro.org> - 2016-05-26 08:00 +0200
  Re: [Regression?] iptables broken on 32bit with pre-4.7-rc Florian Westphal <fw@strlen.de> - 2016-05-26 12:00 +0200
    Re: [Regression?] iptables broken on 32bit with pre-4.7-rc John Stultz <john.stultz@linaro.org> - 2016-05-26 23:10 +0200

csiph-web