From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753459AbcEZJvT (ORCPT ); Thu, 26 May 2016 05:51:19 -0400 Received: from Chamillionaire.breakpoint.cc ([80.244.247.6]:33175 "EHLO Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803AbcEZJvS (ORCPT ); Thu, 26 May 2016 05:51:18 -0400 Date: Thu, 26 May 2016 11:51:12 +0200 From: Florian Westphal To: John Stultz Cc: Florian Westphal , Pablo Neira Ayuso , lkml , netfilter-devel@vger.kernel.org Subject: Re: [Regression?] iptables broken on 32bit with pre-4.7-rc Message-ID: <20160526095112.GA14828@breakpoint.cc> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org John Stultz 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.