Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760601AbXLTCAq (ORCPT ); Wed, 19 Dec 2007 21:00:46 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1760707AbXLTBo2 (ORCPT ); Wed, 19 Dec 2007 20:44:28 -0500 Received: from relay1.sgi.com ([192.48.171.29]:46889 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1760698AbXLTBo0 (ORCPT ); Wed, 19 Dec 2007 20:44:26 -0500 Date: Thu, 20 Dec 2007 12:44:13 +1100 From: David Chinner To: James Morris Cc: David Chinner , lkml , linux-security-module@vger.kernel.org, Eric Paris Subject: Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning Message-ID: <20071220014413.GK4612@sgi.com> References: <20071219093834.GW4396912@sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.1i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3817 Lines: 114 On Thu, Dec 20, 2007 at 11:07:01AM +1100, James Morris wrote: > On Wed, 19 Dec 2007, David Chinner wrote: > > > Folks, > > > > I just updated a git tree and started getting errors on a > > "copy_keys" macro warning. > > > > The code I've been working on uses a ->copy_keys() method for > > copying the keys in a btree block from one place to another. I've > > been working on this code for a while > > (http://oss.sgi.com/archives/xfs/2007-11/msg00046.html) and keep the > > tree I'm working in reletively up to date (lags linus by a couple of > > weeks at most). The update I did this afternoon gave a conflict > > warning with the macro in include/linux/key.h. > > > > Given that I'm not directly including key.h anywhere in the XFS > > code, I'm getting the namespace polluted indirectly from some other > > include that is necessary. > > > > As it turns out, this commit from 13 days ago: > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7cd94146cd504016315608e297219f9fb7b1413b > > > > included security.h in mm.h and that is how I'm seeing the namespace > > poisoning coming from key.h when !CONFIG_KEY. > > > > Including security.h in mm.h means much wider includes for pretty > > much the entire kernel, and it opens up namespace issues like this > > that never previously existed. > > > > The patch below (only tested for !CONFIG_KEYS && !CONFIG_SECURITY) > > moves security.h into the mmap.c and nommu.c files that need it so > > it doesn't end up with kernel wide scope. > > > > Comments? > > The idea with this placement was to keep memory management code with other > similar code, rather than pushing it into security.h, where it does not > functionally belong. > > Something to not also is that you can't "depend" on security.h not being > included all over the place, as LSM does touch a lot of the kernel. > Unecessarily including it is bad, of course. Which is what including it in mm.h does. It also pull sin a lot of other headers files as has already been noted. > I'm not sure I understand your namespace pollution issue, either. doing this globally: #ifdef CONFIG_SOMETHING extern int some_common_name(int a, int b, int c); #else #define some_common_name(a,b,c) 0 #endif means that no-one can use some_common_name *anywhere* in the kernel. In this case, i have a completely *private* use of some_common_name and now I can't use that because the wonderful define above that now has effectively global scope because it gets included from key.h via security.h via mm.h. > In any case, I think the right solution is not to include security.h at > all in mm.h, as it is only being done to get a declaration for > mmap_min_addr. > > How about this, instead ? > > Signed-off-by: James Morris > --- > > mm.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 1b7b95c..02fbac7 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -12,7 +12,6 @@ > #include > #include > #include > -#include > > struct mempolicy; > struct anon_vma; > @@ -34,6 +33,10 @@ extern int sysctl_legacy_va_layout; > #define sysctl_legacy_va_layout 0 > #endif > > +#ifdef CONFIG_SECURITY > +extern unsigned long mmap_min_addr; > +#endif > + > #include > #include > #include Fine by me. Cheers, Dave. -- Dave Chinner Principal Engineer SGI Australian Software Group -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/