Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning

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 <jmorris@xxxxxxxxx>
---

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 <linux/prio_tree.h>
#include <linux/debug_locks.h>
#include <linux/mm_types.h>
-#include <linux/security.h>

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 <asm/page.h>
#include <asm/pgtable.h>
#include <asm/processor.h>

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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages