[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <19723.20449.974043.309608@ipc1.ka-ro>
Date: Fri, 17 Dec 2010 12:56:17 +0100
From: Lothar Waßmann <LW@...O-electronics.de>
To: Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org
Subject: Re: [BUG] 2.6.37-rc5 Memory leak in net/ipv4/udp.c
Hi again,
Lothar Waßmann writes:
> Eric Dumazet writes:
> > 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.
> >
> table->mask is not altered by alloc_large_system_hash(), so you could
> detect the situation beforhand and avoid calling that function in this
> case. As far as I can tell there is no need for
> alloc_large_system_hash() if you later decide to use kmalloc'ed memory
> instead.
>
Forget about this. I was a little confused when reading the
alloc_large_system_hash() function. No I understand.
Sorry for the noise.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@...o-electronics.de
___________________________________________________________
--
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