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: Fri, 24 Feb 2012 20:27:16 -0500 From: Paul Gortmaker <paul.gortmaker@...driver.com> To: Tim Bird <tim.bird@...sony.com> Cc: David Miller <davem@...emloft.net>, kuznet@....inr.ac.ru, linux kernel <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org, eric.dumazet@...il.com Subject: Re: RFC: memory leak in udp_table_init On Fri, Feb 24, 2012 at 7:55 PM, Tim Bird <tim.bird@...sony.com> wrote: > We've uncovered an obscure memory leak in the routine udp_table_init(), > in the file: net/ipv4/udp.c At a glance, I think what you are seeing is the same as this? http://lists.openwall.net/netdev/2011/06/22/12 Paul. > > The allocation sequence is a bit weird, and I've got some questions > about the best way to fix it. > > Here's the code: > > void __init udp_table_init(struct udp_table *table, const char *name) > { > unsigned int i; > > if (!CONFIG_BASE_SMALL) > table->hash = alloc_large_system_hash(name, > 2 * sizeof(struct udp_hslot), > uhash_entries, > 21, /* one slot per 2 MB */ > 0, > &table->log, > &table->mask, > 64 * 1024); > /* > * Make sure hash table has the minimum size > */ > if (CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1) { > table->hash = kmalloc(UDP_HTABLE_SIZE_MIN * > 2 * sizeof(struct udp_hslot), GFP_KERNEL); > if (!table->hash) > panic(name); > table->log = ilog2(UDP_HTABLE_SIZE_MIN); > table->mask = UDP_HTABLE_SIZE_MIN - 1; > } > ... > > We've seen instances where the second allocation of table->hash > is performed, wiping out the first hash table allocation, without a > free. This ends up leaking the previously allocated hash table. > > That is, if we are !CONFIG_BASE_SMALL and for some reason > the alloc_large_system_hash() operation returns less than UDP_HTABLE_SIZE_MIN > hash slots, then it will trigger this. > > There's no complementary "free_large_system_hash()" which can be used to > back out of the first allocation, that I can find. > > We are currently doing the following to avoid the memory leak, but this > seems like it defeats the purpose of checking for the minimum size > (that is, if the first allocation was too small, we don't re-allocate). > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index 5d075b5..2524af4 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -2194,7 +2194,8 @@ void __init udp_table_init(struct udp_table *table, const char *name) > /* > * Make sure hash table has the minimum size > */ > - if (CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1) { > + if ((CONFIG_BASE_SMALL || table->mask < UDP_HTABLE_SIZE_MIN - 1) > + && !table->hash) { > table->hash = kmalloc(UDP_HTABLE_SIZE_MIN * > 2 * sizeof(struct udp_hslot), GFP_KERNEL); > if (!table->hash) > > Any suggestions for a way to correct for a too-small first allocation, without > a memory leak? > > Alternatively - how critical is this UDP_HTABLE_SIZE_MIN for correct operation > of the stack? > > Thanks for any information you can provide. > > -- Tim > > ============================= > Tim Bird > Architecture Group Chair, CE Workgroup of the Linux Foundation > Senior Staff Engineer, Sony Network Entertainment > ============================= > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists