[PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() (try #2)
- From: Jesper Juhl <jesper.juhl@xxxxxxxxx>
- Date: Sat, 18 Mar 2006 21:37:08 +0100
The Coverity checker found that we may leak memory in
mm/slab.c::alloc_kmemlist()
This should fix the leak and coverity bug #589
Currently the only caller of alloc_kmemlist() will BUG() if alloc_kmemlist()
fails, but that doesn't mean we shouldn't clean up properly IMHO. Also, the
caller (do_tune_cpucache()) could maybe be changed in the future to do
something more clever than just BUG() and in that case we really shouldn't
be leaking memory when we return -ENOMEM.
The patch introduces one more loop to the function in the failure path :-(
But, since we are very unlikely to ever hit the failure path this shouldn't
be too painfull.
The patch has been compile and boot tested on x86, but since I'm not very
intimate with the slab code I'd appreciate it if someone would take a close
look on the changes before merging them.
IMO this patch should not go into 2.6.16, but instead spend some time in -mm
with the intention to merge it for 2.6.17 - although it does fix a real leak
it's not a regression compared to 2.6.15.
Signed-off-by: Jesper Juhl <jesper.juhl@xxxxxxxxx>
---
mm/slab.c | 35 +++++++++++++++++++++++++++++------
1 files changed, 29 insertions(+), 6 deletions(-)
--- linux-2.6.16-rc6-mm2-orig/mm/slab.c 2006-03-18 16:55:55.000000000 +0100
+++ linux-2.6.16-rc6-mm2/mm/slab.c 2006-03-18 21:10:56.000000000 +0100
@@ -3399,12 +3399,17 @@ EXPORT_SYMBOL_GPL(kmem_cache_name);
static int alloc_kmemlist(struct kmem_cache *cachep)
{
int node;
+ int count = -1;
struct kmem_list3 *l3;
- int err = 0;
+ struct array_cache *new;
+ struct array_cache **new_alien;
for_each_online_node(node) {
- struct array_cache *nc = NULL, *new;
- struct array_cache **new_alien = NULL;
+ struct array_cache *nc = NULL;
+
+ new = NULL;
+ new_alien = NULL;
+ count++;
#ifdef CONFIG_NUMA
new_alien = alloc_alien_cache(node, cachep->limit);
if (!new_alien)
@@ -3447,10 +3452,28 @@ static int alloc_kmemlist(struct kmem_ca
cachep->batchcount + cachep->num;
cachep->nodelists[node] = l3;
}
- return err;
+ return 0;
+/*
+ If one or more allocations fail we need to undo all allocations done up to
+ this point.
+ Unfortunately this means yet another loop, but since this only happens on
+ failure and frees up memory in a memory-tight situation, it's not too bad.
+ */
fail:
- err = -ENOMEM;
- return err;
+ kfree(new);
+ free_alien_cache(new_alien);
+ for_each_online_node(node) {
+ if (count <= 0)
+ break;
+ if (cachep->nodelists[node]) {
+ kfree(cachep->nodelists[node]->shared);
+ free_alien_cache(cachep->nodelists[node]->alien);
+ kfree(cachep->nodelists[node]);
+ cachep->nodelists[node] = NULL;
+ }
+ count--;
+ }
+ return -ENOMEM;
}
struct ccupdate_struct {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() (try #2)
- From: Pekka Enberg
- Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() (try #2)
- Prev by Date: Re: [patch 1/2] Validate itimer timeval from userspace
- Next by Date: Re: [RFC] Proposed manpage additions for ptrace(2)
- Previous by thread: OOPS: 2.6.16-rc6 cpufreq_conservative
- Next by thread: Re: [PATCH] fix memory leak in mm/slab.c::alloc_kmemlist() (try #2)
- Index(es):
Relevant Pages
|
