From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from youngberry.canonical.com ([91.189.89.112]:54516 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751255AbcFWSTF (ORCPT ); Thu, 23 Jun 2016 14:19:05 -0400 Date: Thu, 23 Jun 2016 19:18:57 +0100 From: Luis Henriques To: gregkh@linuxfoundation.org Cc: fw@strlen.de, pablo@netfilter.org, stable@vger.kernel.org, stable-commits@vger.kernel.org Subject: Re: Patch "netfilter: x_tables: introduce and use xt_copy_counters_from_user" has been added to the 4.4-stable tree Message-ID: <20160623181857.GA32482@ares> References: <14666572377056@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <14666572377056@kroah.com> Sender: stable-owner@vger.kernel.org List-ID: On Wed, Jun 22, 2016 at 09:47:17PM -0700, Greg Kroah-Hartman wrote: > > This is a note to let you know that I've just added the patch titled > > netfilter: x_tables: introduce and use xt_copy_counters_from_user > > to the 4.4-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > The filename of the patch is: > netfilter-x_tables-introduce-and-use-xt_copy_counters_from_user.patch > and it can be found in the queue-4.4 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let know about it. > This patch is not queued for the 3.14 kernel; was this on purpose, or is there a good reason for that? It looks like it's a clean cherry-pick. Cheers, -- Lu�s > > From d7591f0c41ce3e67600a982bab6989ef0f07b3ce Mon Sep 17 00:00:00 2001 > From: Florian Westphal > Date: Fri, 1 Apr 2016 15:37:59 +0200 > Subject: netfilter: x_tables: introduce and use xt_copy_counters_from_user > > From: Florian Westphal > > commit d7591f0c41ce3e67600a982bab6989ef0f07b3ce upstream. > > The three variants use same copy&pasted code, condense this into a > helper and use that. > > Make sure info.name is 0-terminated. > > Signed-off-by: Florian Westphal > Signed-off-by: Pablo Neira Ayuso > Signed-off-by: Greg Kroah-Hartman > > --- > include/linux/netfilter/x_tables.h | 3 + > net/ipv4/netfilter/arp_tables.c | 48 ++---------------------- > net/ipv4/netfilter/ip_tables.c | 48 ++---------------------- > net/ipv6/netfilter/ip6_tables.c | 49 ++---------------------- > net/netfilter/x_tables.c | 74 +++++++++++++++++++++++++++++++++++++ > 5 files changed, 92 insertions(+), 130 deletions(-) > > --- a/include/linux/netfilter/x_tables.h > +++ b/include/linux/netfilter/x_tables.h > @@ -248,6 +248,9 @@ int xt_check_match(struct xt_mtchk_param > int xt_check_target(struct xt_tgchk_param *, unsigned int size, u_int8_t proto, > bool inv_proto); > > +void *xt_copy_counters_from_user(const void __user *user, unsigned int len, > + struct xt_counters_info *info, bool compat); > + > struct xt_table *xt_register_table(struct net *net, > const struct xt_table *table, > struct xt_table_info *bootstrap, > --- a/net/ipv4/netfilter/arp_tables.c > +++ b/net/ipv4/netfilter/arp_tables.c > @@ -1130,55 +1130,17 @@ static int do_add_counters(struct net *n > unsigned int i; > struct xt_counters_info tmp; > struct xt_counters *paddc; > - unsigned int num_counters; > - const char *name; > - int size; > - void *ptmp; > struct xt_table *t; > const struct xt_table_info *private; > int ret = 0; > struct arpt_entry *iter; > unsigned int addend; > -#ifdef CONFIG_COMPAT > - struct compat_xt_counters_info compat_tmp; > > - if (compat) { > - ptmp = &compat_tmp; > - size = sizeof(struct compat_xt_counters_info); > - } else > -#endif > - { > - ptmp = &tmp; > - size = sizeof(struct xt_counters_info); > - } > - > - if (copy_from_user(ptmp, user, size) != 0) > - return -EFAULT; > - > -#ifdef CONFIG_COMPAT > - if (compat) { > - num_counters = compat_tmp.num_counters; > - name = compat_tmp.name; > - } else > -#endif > - { > - num_counters = tmp.num_counters; > - name = tmp.name; > - } > - > - if (len != size + num_counters * sizeof(struct xt_counters)) > - return -EINVAL; > - > - paddc = vmalloc(len - size); > - if (!paddc) > - return -ENOMEM; > - > - if (copy_from_user(paddc, user + size, len - size) != 0) { > - ret = -EFAULT; > - goto free; > - } > + paddc = xt_copy_counters_from_user(user, len, &tmp, compat); > + if (IS_ERR(paddc)) > + return PTR_ERR(paddc); > > - t = xt_find_table_lock(net, NFPROTO_ARP, name); > + t = xt_find_table_lock(net, NFPROTO_ARP, tmp.name); > if (IS_ERR_OR_NULL(t)) { > ret = t ? PTR_ERR(t) : -ENOENT; > goto free; > @@ -1186,7 +1148,7 @@ static int do_add_counters(struct net *n > > local_bh_disable(); > private = t->private; > - if (private->number != num_counters) { > + if (private->number != tmp.num_counters) { > ret = -EINVAL; > goto unlock_up_free; > } > --- a/net/ipv4/netfilter/ip_tables.c > +++ b/net/ipv4/netfilter/ip_tables.c > @@ -1313,55 +1313,17 @@ do_add_counters(struct net *net, const v > unsigned int i; > struct xt_counters_info tmp; > struct xt_counters *paddc; > - unsigned int num_counters; > - const char *name; > - int size; > - void *ptmp; > struct xt_table *t; > const struct xt_table_info *private; > int ret = 0; > struct ipt_entry *iter; > unsigned int addend; > -#ifdef CONFIG_COMPAT > - struct compat_xt_counters_info compat_tmp; > > - if (compat) { > - ptmp = &compat_tmp; > - size = sizeof(struct compat_xt_counters_info); > - } else > -#endif > - { > - ptmp = &tmp; > - size = sizeof(struct xt_counters_info); > - } > - > - if (copy_from_user(ptmp, user, size) != 0) > - return -EFAULT; > - > -#ifdef CONFIG_COMPAT > - if (compat) { > - num_counters = compat_tmp.num_counters; > - name = compat_tmp.name; > - } else > -#endif > - { > - num_counters = tmp.num_counters; > - name = tmp.name; > - } > - > - if (len != size + num_counters * sizeof(struct xt_counters)) > - return -EINVAL; > - > - paddc = vmalloc(len - size); > - if (!paddc) > - return -ENOMEM; > - > - if (copy_from_user(paddc, user + size, len - size) != 0) { > - ret = -EFAULT; > - goto free; > - } > + paddc = xt_copy_counters_from_user(user, len, &tmp, compat); > + if (IS_ERR(paddc)) > + return PTR_ERR(paddc); > > - t = xt_find_table_lock(net, AF_INET, name); > + t = xt_find_table_lock(net, AF_INET, tmp.name); > if (IS_ERR_OR_NULL(t)) { > ret = t ? PTR_ERR(t) : -ENOENT; > goto free; > @@ -1369,7 +1331,7 @@ do_add_counters(struct net *net, const v > > local_bh_disable(); > private = t->private; > - if (private->number != num_counters) { > + if (private->number != tmp.num_counters) { > ret = -EINVAL; > goto unlock_up_free; > } > --- a/net/ipv6/netfilter/ip6_tables.c > +++ b/net/ipv6/netfilter/ip6_tables.c > @@ -1325,55 +1325,16 @@ do_add_counters(struct net *net, const v > unsigned int i; > struct xt_counters_info tmp; > struct xt_counters *paddc; > - unsigned int num_counters; > - char *name; > - int size; > - void *ptmp; > struct xt_table *t; > const struct xt_table_info *private; > int ret = 0; > struct ip6t_entry *iter; > unsigned int addend; > -#ifdef CONFIG_COMPAT > - struct compat_xt_counters_info compat_tmp; > > - if (compat) { > - ptmp = &compat_tmp; > - size = sizeof(struct compat_xt_counters_info); > - } else > -#endif > - { > - ptmp = &tmp; > - size = sizeof(struct xt_counters_info); > - } > - > - if (copy_from_user(ptmp, user, size) != 0) > - return -EFAULT; > - > -#ifdef CONFIG_COMPAT > - if (compat) { > - num_counters = compat_tmp.num_counters; > - name = compat_tmp.name; > - } else > -#endif > - { > - num_counters = tmp.num_counters; > - name = tmp.name; > - } > - > - if (len != size + num_counters * sizeof(struct xt_counters)) > - return -EINVAL; > - > - paddc = vmalloc(len - size); > - if (!paddc) > - return -ENOMEM; > - > - if (copy_from_user(paddc, user + size, len - size) != 0) { > - ret = -EFAULT; > - goto free; > - } > - > - t = xt_find_table_lock(net, AF_INET6, name); > + paddc = xt_copy_counters_from_user(user, len, &tmp, compat); > + if (IS_ERR(paddc)) > + return PTR_ERR(paddc); > + t = xt_find_table_lock(net, AF_INET6, tmp.name); > if (IS_ERR_OR_NULL(t)) { > ret = t ? PTR_ERR(t) : -ENOENT; > goto free; > @@ -1381,7 +1342,7 @@ do_add_counters(struct net *net, const v > > local_bh_disable(); > private = t->private; > - if (private->number != num_counters) { > + if (private->number != tmp.num_counters) { > ret = -EINVAL; > goto unlock_up_free; > } > --- a/net/netfilter/x_tables.c > +++ b/net/netfilter/x_tables.c > @@ -751,6 +751,80 @@ int xt_check_target(struct xt_tgchk_para > } > EXPORT_SYMBOL_GPL(xt_check_target); > > +/** > + * xt_copy_counters_from_user - copy counters and metadata from userspace > + * > + * @user: src pointer to userspace memory > + * @len: alleged size of userspace memory > + * @info: where to store the xt_counters_info metadata > + * @compat: true if we setsockopt call is done by 32bit task on 64bit kernel > + * > + * Copies counter meta data from @user and stores it in @info. > + * > + * vmallocs memory to hold the counters, then copies the counter data > + * from @user to the new memory and returns a pointer to it. > + * > + * If @compat is true, @info gets converted automatically to the 64bit > + * representation. > + * > + * The metadata associated with the counters is stored in @info. > + * > + * Return: returns pointer that caller has to test via IS_ERR(). > + * If IS_ERR is false, caller has to vfree the pointer. > + */ > +void *xt_copy_counters_from_user(const void __user *user, unsigned int len, > + struct xt_counters_info *info, bool compat) > +{ > + void *mem; > + u64 size; > + > +#ifdef CONFIG_COMPAT > + if (compat) { > + /* structures only differ in size due to alignment */ > + struct compat_xt_counters_info compat_tmp; > + > + if (len <= sizeof(compat_tmp)) > + return ERR_PTR(-EINVAL); > + > + len -= sizeof(compat_tmp); > + if (copy_from_user(&compat_tmp, user, sizeof(compat_tmp)) != 0) > + return ERR_PTR(-EFAULT); > + > + strlcpy(info->name, compat_tmp.name, sizeof(info->name)); > + info->num_counters = compat_tmp.num_counters; > + user += sizeof(compat_tmp); > + } else > +#endif > + { > + if (len <= sizeof(*info)) > + return ERR_PTR(-EINVAL); > + > + len -= sizeof(*info); > + if (copy_from_user(info, user, sizeof(*info)) != 0) > + return ERR_PTR(-EFAULT); > + > + info->name[sizeof(info->name) - 1] = '\0'; > + user += sizeof(*info); > + } > + > + size = sizeof(struct xt_counters); > + size *= info->num_counters; > + > + if (size != (u64)len) > + return ERR_PTR(-EINVAL); > + > + mem = vmalloc(len); > + if (!mem) > + return ERR_PTR(-ENOMEM); > + > + if (copy_from_user(mem, user, len) == 0) > + return mem; > + > + vfree(mem); > + return ERR_PTR(-EFAULT); > +} > +EXPORT_SYMBOL_GPL(xt_copy_counters_from_user); > + > #ifdef CONFIG_COMPAT > int xt_compat_target_offset(const struct xt_target *target) > { > > > Patches currently in stable-queue which might be from fw@strlen.de are > > queue-4.4/netfilter-x_tables-validate-targets-of-jumps.patch > queue-4.4/netfilter-x_tables-introduce-and-use-xt_copy_counters_from_user.patch > queue-4.4/netfilter-arp_tables-simplify-translate_compat_table-args.patch > queue-4.4/netfilter-x_tables-validate-e-target_offset-early.patch > queue-4.4/netfilter-x_tables-assert-minimum-target-size.patch > queue-4.4/netfilter-ip6_tables-simplify-translate_compat_table-args.patch > queue-4.4/netfilter-x_tables-check-for-bogus-target-offset.patch > queue-4.4/netfilter-x_tables-add-compat-version-of-xt_check_entry_offsets.patch > queue-4.4/netfilter-x_tables-don-t-reject-valid-target-size-on-some-architectures.patch > queue-4.4/netfilter-x_tables-check-standard-target-size-too.patch > queue-4.4/netfilter-x_tables-xt_compat_match_from_user-doesn-t-need-a-retval.patch > queue-4.4/netfilter-x_tables-do-compat-validation-via-translate_table.patch > queue-4.4/netfilter-x_tables-add-and-use-xt_check_entry_offsets.patch > queue-4.4/netfilter-x_tables-don-t-move-to-non-existent-next-rule.patch > queue-4.4/netfilter-x_tables-kill-check_entry-helper.patch > queue-4.4/netfilter-x_tables-validate-all-offsets-and-sizes-in-a-rule.patch > queue-4.4/netfilter-x_tables-make-sure-e-next_offset-covers-remaining-blob-size.patch > queue-4.4/netfilter-ip_tables-simplify-translate_compat_table-args.patch > queue-4.4/netfilter-x_tables-fix-unconditional-helper.patch > -- > To unsubscribe from this list: send the line "unsubscribe stable" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html