Message ID | 1461332394-3994-8-git-send-email-pablo@netfilter.org |
---|---|
State | Awaiting Upstream |
Delegated to: | Pablo Neira |
Headers | show |
Pablo Neira Ayuso <pablo@netfilter.org> writes: > From: Florian Westphal <fw@strlen.de> > > We have targets and standard targets -- the latter carries a verdict. > > The ip/ip6tables validation functions will access t->verdict for the > standard targets to fetch the jump offset or verdict for chainloop > detection, but this happens before the targets get checked/validated. > > Thus we also need to check for verdict presence here, else t->verdict > can point right after a blob. > > Spotted with UBSAN while testing malformed blobs. This breaks iptables on PPC32. # iptables -nL iptables v1.4.21: can't initialize iptables table `filter': Table does not exist (do you need to insmod?) Perhaps iptables or your kernel needs to be upgraded. # modprobe iptable-filter FATAL: Error inserting iptable_filter (/lib/modules/4.7.0-rc1/kernel/net/ipv4/netfilter/iptable_filter.ko): Invalid argument Andreas.
Andreas Schwab <schwab@linux-m68k.org> wrote: > > From: Florian Westphal <fw@strlen.de> > > > > We have targets and standard targets -- the latter carries a verdict. > > > > The ip/ip6tables validation functions will access t->verdict for the > > standard targets to fetch the jump offset or verdict for chainloop > > detection, but this happens before the targets get checked/validated. > > > > Thus we also need to check for verdict presence here, else t->verdict > > can point right after a blob. > > > > Spotted with UBSAN while testing malformed blobs. > > This breaks iptables on PPC32. Yes, we got bug report for arm32, I'm sorry about this -- only 32bit platform I tested was i686 and that only needs 4byte alignment for u64. This fix should help: https://git.kernel.org/cgit/linux/kernel/git/pablo/nf.git/commit/?id=7b7eba0f3515fca3296b8881d583f7c1042f5226 -- 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
On Mon, Jun 06, 2016 at 12:02:10AM +0200, Florian Westphal wrote: > Andreas Schwab <schwab@linux-m68k.org> wrote: > > > From: Florian Westphal <fw@strlen.de> > > > > > > We have targets and standard targets -- the latter carries a verdict. > > > > > > The ip/ip6tables validation functions will access t->verdict for the > > > standard targets to fetch the jump offset or verdict for chainloop > > > detection, but this happens before the targets get checked/validated. > > > > > > Thus we also need to check for verdict presence here, else t->verdict > > > can point right after a blob. > > > > > > Spotted with UBSAN while testing malformed blobs. > > > > This breaks iptables on PPC32. > > Yes, we got bug report for arm32, I'm sorry about this -- only 32bit > platform I tested was i686 and that only needs 4byte alignment for u64. > > This fix should help: > > https://git.kernel.org/cgit/linux/kernel/git/pablo/nf.git/commit/?id=7b7eba0f3515fca3296b8881d583f7c1042f5226 Short notice: Will be handing over this in a pull request for David at some point today. -- 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 --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index fa206ce..1cb7a27 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -540,6 +540,13 @@ int xt_compat_match_to_user(const struct xt_entry_match *m, } EXPORT_SYMBOL_GPL(xt_compat_match_to_user); +/* non-compat version may have padding after verdict */ +struct compat_xt_standard_target { + struct compat_xt_entry_target t; + compat_uint_t verdict; +}; + +/* see xt_check_entry_offsets */ int xt_compat_check_entry_offsets(const void *base, unsigned int target_offset, unsigned int next_offset) @@ -557,6 +564,10 @@ int xt_compat_check_entry_offsets(const void *base, if (target_offset + t->u.target_size > next_offset) return -EINVAL; + if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 && + target_offset + sizeof(struct compat_xt_standard_target) != next_offset) + return -EINVAL; + return 0; } EXPORT_SYMBOL(xt_compat_check_entry_offsets); @@ -596,6 +607,10 @@ int xt_check_entry_offsets(const void *base, if (target_offset + t->u.target_size > next_offset) return -EINVAL; + if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 && + target_offset + sizeof(struct xt_standard_target) != next_offset) + return -EINVAL; + return 0; } EXPORT_SYMBOL(xt_check_entry_offsets);