From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613 Date: Fri, 20 Feb 2009 14:28:07 -0800 Message-ID: <20090220142807.a28734a8.akpm@linux-foundation.org> References: <20090220122242.b36a778f.akpm@linux-foundation.org> <1235168219.3019.4.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from smtp1.linux-foundation.org ([140.211.169.13]:59340 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753471AbZBTW3O (ORCPT ); Fri, 20 Feb 2009 17:29:14 -0500 In-Reply-To: <1235168219.3019.4.camel@localhost.localdomain> Sender: linux-next-owner@vger.kernel.org List-ID: To: Mimi Zohar Cc: a.beregalov@gmail.com, linux-kernel@vger.kernel.org, linux-next@vger.kernel.org, xfs@oss.sgi.com, jmorris@namei.org On Fri, 20 Feb 2009 17:16:59 -0500 Mimi Zohar wrote: > integrity: ima iint radix_tree_lookup locking fix > > Based on Andrew Morton's comments: > - add missing locks around radix_tree_lookup in ima_iint_insert() > > Signed-off-by: Mimi Zohar > > Index: security-testing-2.6/security/integrity/ima/ima_iint.c > =================================================================== > --- security-testing-2.6.orig/security/integrity/ima/ima_iint.c > +++ security-testing-2.6/security/integrity/ima/ima_iint.c > @@ -73,8 +73,10 @@ out: > if (rc < 0) { > kmem_cache_free(iint_cache, iint); > if (rc == -EEXIST) { > + spin_lock(&ima_iint_lock); > iint = radix_tree_lookup(&ima_iint_store, > (unsigned long)inode); > + spin_unlock(&ima_iint_lock); > } else > iint = NULL; > } Can the -EEXIST ever actually happen? On the inode_init_always() path (at least), I don't think that any other thread of control can have access to this inode*, so there is no way in which a race can result in someone else adding this inode first? Also, idle question: why does the radix tree exist at all? Would it have been possible to just add a `struct ima_iint_cache *' field to the inode instead?