netdev - Re: [BUG] 2.6.37-rc5 Memory leak in net/ipv4/udp.c

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1292585534.2906.12.camel@edumazet-laptop>
Date:	Fri, 17 Dec 2010 12:32:14 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Lothar Waßmann <LW@...O-electronics.de>
Cc:	netdev@...r.kernel.org
Subject: Re: [BUG] 2.6.37-rc5 Memory leak in net/ipv4/udp.c

Le vendredi 17 décembre 2010 à 12:11 +0100, Lothar Waßmann a écrit :
> Hi,
> 
> Eric Dumazet writes:
> > Le vendredi 17 décembre 2010 à 11:18 +0100, Lothar Waßmann a écrit :
> > > The offending code in net/ipv4/udp.c is:
> > > |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);
> > > In case of !CONFIG_BASE_SMALL and 'table->mask < UDP_HTABLE_SIZE_MIN - 1)'
> > > the memory allocated in the previous if clause becomes inacessible!
> > > 
> > > Shouldn't this be:
> > > |	if (!CONFIG_BASE_SMALL && table->mask >= UDP_HTABLE_SIZE_MIN - 1) {
> > > |		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);
> > > |	} else {
> > > |		table->hash = kmalloc(UDP_HTABLE_SIZE_MIN *
> > > |				      2 * sizeof(struct udp_hslot), GFP_KERNEL);
> > > [...]
> > > 
> > 
> > Nothing we can do about it, there is no API to reverse the
> > alloc_large_system_hash() effect. We could call kmemleak api to at least
> > avoid this false alarm.
> > 
> Do you have to call it at all in case of table->mask < UDP_HTABLE_SIZE_MIN - 1?
> 

We call alloc_large_system_hash() asking it to size the table _itself_.
We give some hints : 

- How many slots per MB of avail memory.
- An upper limit (64*1024 slots because we only handle 65536 udp ports)
- but not a lower limit (not available in the API)

Problem is in your case, alloc_large_system_hash() allocates a very
small area. Then we catch the problem, seeing table->mask is too small
for our needs. We prefer to 'lost' this too small memory than crashing
kernel later.


> > We really want a minimum size for the UDP hash table, because our algos
> > depend on this.
> > 
> I can't see why this could not be achieved by doing _either_
> alloc_large_system_hash() _OR_ kmalloc() as stated above, but not
> both.

We definitly want alloc_large_system_hash() for the general case
(nice NUMA spread, while kmalloc() would allocate the hash table on a
single memory node. Not so nice)

One way to handle this problem would be to add a new parameter to
alloc_large_system_hash() to specify a lower limit.
alloc_large_system_hash() would not even try to allocate a too small
array.

Is your system so small ?



--
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