lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 May 2018 12:09:08 +0200 From: Greg Kroah-Hartman <gregkh@...uxfoundation.org> To: Eric Dumazet <eric.dumazet@...il.com>, Greg Hackmann <ghackmann@...gle.com> Cc: Pablo Neira Ayuso <pablo@...filter.org>, Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>, Florian Westphal <fw@...len.de>, Michal Kubecek <mkubecek@...e.cz>, netfilter-devel@...r.kernel.org, coreteam@...filter.org, netdev@...r.kernel.org Subject: Re: [PATCH v2] netfilter: properly initialize xt_table_info structure On Thu, May 17, 2018 at 02:55:42AM -0700, Eric Dumazet wrote: > > > On 05/17/2018 02:34 AM, Greg Kroah-Hartman wrote: > > When allocating a xt_table_info structure, we should be clearing out the > > full amount of memory that was allocated, not just the "header" of the > > structure. Otherwise odd values could be passed to userspace, which is > > not a good thing. > > > > Cc: stable <stable@...r.kernel.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org> > > --- > > v2: use kvzalloc instead of kvmalloc/memset pair, as suggested by Michal Kubecek > > > > net/netfilter/x_tables.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c > > index cb7cb300c3bc..cd22bb9b66f3 100644 > > --- a/net/netfilter/x_tables.c > > +++ b/net/netfilter/x_tables.c > > @@ -1183,11 +1183,10 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size) > > * than shoot all processes down before realizing there is nothing > > * more to reclaim. > > */ > > - info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY); > > + info = kvzalloc(sz, GFP_KERNEL | __GFP_NORETRY); > > if (!info) > > return NULL; > > > > - memset(info, 0, sizeof(*info)); > > info->size = size; > > return info; > > } > > > > I am curious, what particular path does not later overwrite the whole zone ? The path back was long, adding Greg Hackman who helped to debug this to the To: to confirm that I got this correct... In do_ipt_get_ctl, the IPT_SO_GET_ENTRIES: option uses a len value that can be larger than the size of the structure itself. Then the data is copied to userspace in copy_entries_to_user() for ipv4 and v6, and that's where the "bad data" was noticed (a researcher was using a kernel patch to determine what the data was) Greg, that's the correct path here, right? > Do not get me wrong, this is not fast path, but these blobs can be huge. Yeah, I bet, but for "normal" cases the size should be small and all should be fine. thanks, greg k-h
Powered by blists - more mailing lists