linux-kernel - Re: [PATCH 2.6.28-rc5 01/11] kmemleak: Add the base support

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: <1227269248.7015.31.camel@pc1117.cambridge.arm.com>
Date:	Fri, 21 Nov 2008 12:07:28 +0000
From:	Catalin Marinas <catalin.marinas@....com>
To:	Pekka Enberg <penberg@...helsinki.fi>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2.6.28-rc5 01/11] kmemleak: Add the base support

On Thu, 2008-11-20 at 21:35 +0200, Pekka Enberg wrote:
> On Thu, Nov 20, 2008 at 1:30 PM, Catalin Marinas
> <catalin.marinas@....com> wrote:
> > +#ifdef CONFIG_SMP
> > +#define cache_line_align(x)    L1_CACHE_ALIGN(x)
> > +#else
> > +#define cache_line_align(x)    (x)
> > +#endif
> 
> Maybe we should be put to <linux/cache.h> and call it cache_line_align_in_smp()?

Yes, it makes sense.

> > +/*
> > + * Object allocation
> > + */
> > +static void *fast_cache_alloc(struct fast_cache *cache)
[...]
> The slab allocators are pretty fast as well. Is there a reason you
> can't use kmalloc() or kmem_cache_alloc() for this? 

The reason for the internal allocator wasn't speed, I would be happy to
use the main one. The past kmemleak versions used the slab allocator and
I was getting lockdep reports about the l3->list_lock via the
cache_grow() and memleak_alloc() functions, IIRC. At that time I also
had another lock held during radix_tree_insert (this function calling
kmem_cache_alloc), so some of these problems might have gone away now
with a finer-grained locking and some RCU usage (actually no locks
should be held when calling the alloc functions from kmemleak).

It seems that the flags are propagated inside the slab allocator so it
is also possible to miss some slab-internal allocations we would like to
track (like slabmgmt objects in a list) or get too many recursive calls
via memleak_alloc().

> You can fix the
> recursion problem by adding a new GFP_NOLEAKTRACK flag that makes sure
> memleak hooks are not invoked if it's set.

But this flag doesn't get passed to kfree. An option would be to use a
kmem_cache and a SLAB_NOLEAKTRACE bit so that it can be checked via
kmem_cache_free().

If you don't think the above issues are real problems, I'm happy to give
it a try.

-- 
Catalin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/