linux-kernel - RE: [PATCH] slub: Proper kmemleak tracking if CONFIG_SLUB

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 for Android: free password hash cracker in your pocket

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

Message-ID: <F5184659D418E34EA12B1903EE5EF5FD8538E86615@seldmbx02.corpusers.net>
Date:	Wed, 2 Oct 2013 17:33:47 +0200
From:	"Bird, Tim" <Tim.Bird@...ymobile.com>
To:	Christoph Lameter <cl@...ux.com>,
	Frank Rowand <frowand.list@...il.com>
CC:	Pekka Enberg <penberg@...nel.org>, Matt Mackall <mpm@...enic.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Linux Kernel list <linux-kernel@...r.kernel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	"Bobniev, Roman" <Roman.Bobniev@...ymobile.com>,
	"Andersson, Björn" 
	<Bjorn.Andersson@...ymobile.com>
Subject: RE: [PATCH] slub: Proper kmemleak tracking if CONFIG_SLUB_DEBUG
 disabled

On Wednesday, October 02, 2013 7:41 AM, Christoph Lameter [cl@...ux.com] wrote:
>
>On Fri, 27 Sep 2013, Frank Rowand wrote:
>
>> Move the kmemleak code for small block allocation out from
>> under CONFIG_SLUB_DEBUG.
>
>Well in that case it may be better to move the hooks as a whole out of
>the CONFIG_SLUB_DEBUG section. Do the #ifdeffering for each call from the
>hooks instead.
>
>The point of the hook functions is to separate the hooks out of the
>functions so taht they do not accumulate in the main code.
>
>The patch moves one hook back into the main code. Please keep the checks
>in the hooks.

Thanks for the feedback.  Roman's first patch, which we discussed internally
before sending this one, did exactly that.  I guess Roman gets to say "I told
you so." :-)  My bad for telling him to change it.

We'll refactor along the lines that you describe, and send another one.

The problem child is actually the unconditional call to kmemleak_alloc()
in kmalloc_large_node() (in slub.c).  The problem comes because that call
is unconditional on CONFIG_SLUB_DEBUG but the kmemleak
calls in the hook routines are conditional on CONFIG_SLUB_DEBUG.
So if you have CONFIG_SLUB_DEBUG=n but CONFIG_DEBUG_KMEMLEAK=y,
you get the false reports.

Now, there are kmemleak calls in kmalloc_large_node() and kfree() that don't
follow the "hook" pattern.  Should these be moved to 'hook' routines, to keep
all the checks in the hooks?

Personally, I like the idea of keeping bookeeping/tracing/debug stuff in hook
routines.  I also like de-coupling CONFIG_SLUB_DEBUG and CONFIG_DEBUG_KMEMLEAK,
but maybe others have a different opinon.  Unless someone speaks up, we'll
move the the currently in-function kmemleak calls into hooks, and all of the
kmemleak stuff out from under CONFIG_SLUB_DEBUG.
We'll have to see if the ifdefs get a little messy.
  -- Tim


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