linux-kernel - Re: kfree(0)

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: <alpine.LFD.0.999.0708150454480.6482@enigma.security.iitk.ac.in>
Date:	Wed, 15 Aug 2007 05:12:41 +0530 (IST)
From:	Satyam Sharma <satyam@...radead.org>
To:	Arjan van de Ven <arjan@...radead.org>
cc:	Tim Bird <tim.bird@...sony.com>,
	linux kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <clameter@....com>
Subject: Re: kfree(0) - ok?



On Tue, 14 Aug 2007, Arjan van de Ven wrote:

> 
> On Tue, 2007-08-14 at 15:59 -0700, Tim Bird wrote:
> > Hi all,
> > 
> > I have a quick question.
> > 
> > I'm trying to resurrect a patch from the Linux-tiny patch suite,
> > to do accounting of kmalloc memory allocations.  In testing it
> > with Linux 2.6.22, I've found a large number of kfrees of
> > NULL pointers.
> > 
> > Is this considered OK?  Or should I examine the offenders
> > to see if something is coded badly?
> 
> kfree(NULL) is explicitly ok and it saves code size to not check
> anywhere

But that doesn't come free of cost, does it, seeing we've now pushed
the conditional inside kfree() itself. kfree() isn't inlined so we do
save on space but lose out on the extra time overhead for the common
case. Speaking of which ...

[PATCH] {slub, slob}: use unlikely() for kfree(ZERO_OR_NULL_PTR) check

Considering kfree(NULL) would normally occur only in error paths and
kfree(ZERO_SIZE_PTR) is uncommon as well, so let's use unlikely() for
the condition check in SLUB's and SLOB's kfree() to optimize for the
common case. SLAB has this already.

Signed-off-by: Satyam Sharma <satyam@...radead.org>

---

 mm/slob.c |    2 +-
 mm/slub.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/slob.c b/mm/slob.c
index ec33fcd..37a8b9a 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -466,7 +466,7 @@ void kfree(const void *block)
 {
 	struct slob_page *sp;
 
-	if (ZERO_OR_NULL_PTR(block))
+	if (unlikely(ZERO_OR_NULL_PTR(block)))
 		return;
 
 	sp = (struct slob_page *)virt_to_page(block);
diff --git a/mm/slub.c b/mm/slub.c
index 69d02e3..3788537 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2467,7 +2467,7 @@ void kfree(const void *x)
 	 * this comparison would be true for all "negative" pointers
 	 * (which would cover the whole upper half of the address space).
 	 */
-	if (ZERO_OR_NULL_PTR(x))
+	if (unlikely(ZERO_OR_NULL_PTR(x)))
 		return;
 
 	page = virt_to_head_page(x);
-
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/