linux-kernel - Re: [WARNING] lockdep and kmemcheck_alloc

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.DEB.2.00.1205201405110.10189@chino.kir.corp.google.com>
Date:	Sun, 20 May 2012 14:10:08 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Ben Hutchings <ben@...adent.org.uk>,
	Steven Rostedt <rostedt@...dmis.org>
cc:	Christoph Lameter <cl@...ux.com>,
	LKML <linux-kernel@...r.kernel.org>,
	stable <stable@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [WARNING] lockdep and kmemcheck_alloc_shadow

On Sun, 20 May 2012, Ben Hutchings wrote:

> Seems to be a bug in SLUB:
> 
> > static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> > {
> [...]
> > 	if (flags & __GFP_WAIT)
> > 		local_irq_disable();
> > 
> > 	if (!page)
> > 		return NULL;
> > 
> > 	if (kmemcheck_enabled
> > 		&& !(s->flags & (SLAB_NOTRACK | DEBUG_DEFAULT_FLAGS))) {
> > 		int pages = 1 << oo_order(oo);
> > 
> > 		kmemcheck_alloc_shadow(page, oo_order(oo), flags, node);
> [...]
> 
> So kmemcheck_alloc_shadow() runs with IRQs off, but uses the same flags
> as the original allocation.
> 
> Looks like this was introduced by:
> 
> commit 7e0528dadc9f8b04e4de0dba48a075100c2afe75
> Author: Christoph Lameter <cl@...ux.com>
> Date:   Wed Jun 1 12:25:44 2011 -0500
> 
>     slub: Push irq disable into allocate_slab()
> 
> which went into 3.1.
> 

Agreed, slab handles this correctly and it looks like slub ends up 
disabling irqs too early.

Does this fix it?  If so, we'll need to annotate it for stable as Ben 
noted.


mm, slub: ensure irqs are not disabled for kmemcheck allocation

kmemcheck_alloc_shadow() cannot be called with irqs disabled, which is
possible with __GFP_WAIT.  Ensure we always have them enabled at
allocation time like slab does.

Reported-by: Steven Rostedt <rostedt@...dmis.org>
Suggested-by: Ben Hutchings <ben@...adent.org.uk>
Signed-off-by: David Rientjes <rientjes@...gle.com>
---
diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1314,13 +1314,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 			stat(s, ORDER_FALLBACK);
 	}
 
-	if (flags & __GFP_WAIT)
-		local_irq_disable();
-
-	if (!page)
-		return NULL;
-
-	if (kmemcheck_enabled
+	if (page && kmemcheck_enabled
 		&& !(s->flags & (SLAB_NOTRACK | DEBUG_DEFAULT_FLAGS))) {
 		int pages = 1 << oo_order(oo);
 
@@ -1336,6 +1330,11 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
+	if (flags & __GFP_WAIT)
+		local_irq_disable();
+	if (!page)
+		return NULL;
+
 	page->objects = oo_objects(oo);
 	mod_zone_page_state(page_zone(page),
 		(s->flags & SLAB_RECLAIM_ACCOUNT) ?
--
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/