2008-08-08 18:56:07

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 2/4] integrity: special fs magic

- Move special fs magic number definitions to magic.h
- Add magic.h include

Signed-off-by: Mimi Zohar <[email protected]>

Reviewed-by: James Morris <[email protected]>

Index: security-testing-2.6/fs/debugfs/inode.c
===================================================================
--- security-testing-2.6.orig/fs/debugfs/inode.c
+++ security-testing-2.6/fs/debugfs/inode.c
@@ -26,8 +26,7 @@
#include <linux/debugfs.h>
#include <linux/fsnotify.h>
#include <linux/string.h>
-
-#define DEBUGFS_MAGIC 0x64626720
+#include <linux/magic.h>

static struct vfsmount *debugfs_mount;
static int debugfs_mount_count;
Index: security-testing-2.6/include/linux/magic.h
===================================================================
--- security-testing-2.6.orig/include/linux/magic.h
+++ security-testing-2.6/include/linux/magic.h
@@ -6,6 +6,10 @@
#define AFS_SUPER_MAGIC 0x5346414F
#define AUTOFS_SUPER_MAGIC 0x0187
#define CODA_SUPER_MAGIC 0x73757245
+#define DEBUGFS_MAGIC 0x64626720
+#define SYSFS_MAGIC 0x62656572
+#define SECURITYFS_MAGIC 0x73636673
+#define TMPFS_MAGIC 0x01021994
#define EFS_SUPER_MAGIC 0x414A53
#define EXT2_SUPER_MAGIC 0xEF53
#define EXT3_SUPER_MAGIC 0xEF53
Index: security-testing-2.6/mm/shmem.c
===================================================================
--- security-testing-2.6.orig/mm/shmem.c
+++ security-testing-2.6/mm/shmem.c
@@ -50,14 +50,12 @@
#include <linux/migrate.h>
#include <linux/highmem.h>
#include <linux/seq_file.h>
+#include <linux/magic.h>

#include <asm/uaccess.h>
#include <asm/div64.h>
#include <asm/pgtable.h>

-/* This magic number is used in glibc for posix shared memory */
-#define TMPFS_MAGIC 0x01021994
-
#define ENTRIES_PER_PAGE (PAGE_CACHE_SIZE/sizeof(unsigned long))
#define ENTRIES_PER_PAGEPAGE (ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)
#define BLOCKS_PER_PAGE (PAGE_CACHE_SIZE/512)
Index: security-testing-2.6/security/inode.c
===================================================================
--- security-testing-2.6.orig/security/inode.c
+++ security-testing-2.6/security/inode.c
@@ -20,8 +20,7 @@
#include <linux/init.h>
#include <linux/namei.h>
#include <linux/security.h>
-
-#define SECURITYFS_MAGIC 0x73636673
+#include <linux/magic.h>

static struct vfsmount *mount;
static int mount_count;

--


2008-08-08 19:07:59

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> - Move special fs magic number definitions to magic.h
> - Add magic.h include
>
> Signed-off-by: Mimi Zohar <[email protected]>

Why? What is this patch for? Are you going to do something with these
magic values later?

thanks,

greg k-h

2008-08-08 19:17:46

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

On Fri, Aug 08, 2008 at 12:04:48PM -0700, Greg KH wrote:
> On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > - Move special fs magic number definitions to magic.h
> > - Add magic.h include
> >
> > Signed-off-by: Mimi Zohar <[email protected]>
>
> Why? What is this patch for? Are you going to do something with these
> magic values later?

Ok, I now see the follow-up patch that does something with them.

You should say so in this patch.

And is it really ok to be doing things from userspace based on a
filesystem "magic" key? Those are numbers we have never exported to
userspace before, what happens if they are changed?

thanks,

greg k-h

2008-08-08 19:36:19

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

[email protected] wrote on 08/08/2008 03:04:48
PM:


> Re: [PATCH 2/4] integrity: special fs magic
>
> On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > - Move special fs magic number definitions to magic.h
> > - Add magic.h include
> >
> > Signed-off-by: Mimi Zohar <[email protected]>
>
> Why? What is this patch for? Are you going to do something with these
> magic values later?
>
> thanks,
>

yes, [PATCH 4/4] integrity: IMA as an integrity service provider,
uses these definition in defining the default IMA measurement policy.
The default policy can be replaced with an LSM specific one. More
details on the IMA measurement policy are available in
Documentation/ABI/testing/ima_policy.

Mimi


2008-08-08 19:50:19

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

Greg KH <[email protected]> wrote on 08/08/2008 03:15:19 PM:

> On Fri, Aug 08, 2008 at 12:04:48PM -0700, Greg KH wrote:
> > On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > > - Move special fs magic number definitions to magic.h
> > > - Add magic.h include
> > >
> > > Signed-off-by: Mimi Zohar <[email protected]>
> >
> > Why? What is this patch for? Are you going to do something with
these
> > magic values later?
>
> Ok, I now see the follow-up patch that does something with them.
>
> You should say so in this patch.
>
> And is it really ok to be doing things from userspace based on a
> filesystem "magic" key? Those are numbers we have never exported to
> userspace before, what happens if they are changed?
>
> thanks,
>
> greg k-h

Userspace only loads the measurement policy (via securityfs), and if
a magic number changes, and the policy is not updated to match, then
we would end up measuring some filesystems we didn't need to. Before
the magic numbers were hard coded in IMA, now at least, it's extensible.

Mimi

2008-08-08 23:16:00

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

On Fri, Aug 08, 2008 at 03:50:02PM -0400, Mimi Zohar wrote:
> Greg KH <[email protected]> wrote on 08/08/2008 03:15:19 PM:
>
> > On Fri, Aug 08, 2008 at 12:04:48PM -0700, Greg KH wrote:
> > > On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > > > - Move special fs magic number definitions to magic.h
> > > > - Add magic.h include
> > > >
> > > > Signed-off-by: Mimi Zohar <[email protected]>
> > >
> > > Why? What is this patch for? Are you going to do something with
> these
> > > magic values later?
> >
> > Ok, I now see the follow-up patch that does something with them.
> >
> > You should say so in this patch.
> >
> > And is it really ok to be doing things from userspace based on a
> > filesystem "magic" key? Those are numbers we have never exported to
> > userspace before, what happens if they are changed?
> >
> > thanks,
> >
> > greg k-h
>
> Userspace only loads the measurement policy (via securityfs), and if
> a magic number changes, and the policy is not updated to match, then
> we would end up measuring some filesystems we didn't need to. Before
> the magic numbers were hard coded in IMA, now at least, it's extensible.

Why not just use a name then instead? That way if the number changes
within the kernel, then it will always be right.

thanks,

greg k-h

2008-08-08 23:16:24

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> - Move special fs magic number definitions to magic.h
> - Add magic.h include

Moving them is fine, using them for anything however is not.

2008-08-09 18:48:16

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

On Fri, Aug 08, 2008 at 12:15:19PM -0700, Greg KH wrote:
> On Fri, Aug 08, 2008 at 12:04:48PM -0700, Greg KH wrote:
> > On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > > - Move special fs magic number definitions to magic.h
> > > - Add magic.h include
> > >
> > > Signed-off-by: Mimi Zohar <[email protected]>
> >
> > Why? What is this patch for? Are you going to do something with these
> > magic values later?
>
> Ok, I now see the follow-up patch that does something with them.
>
> You should say so in this patch.
>
> And is it really ok to be doing things from userspace based on a
> filesystem "magic" key? Those are numbers we have never exported to
> userspace before, what happens if they are changed?

These constants re exported to userspace in struct statfs .type and
better don't change. Providing the symbolic names for them sounds
like a good idea to me. What userspace does with that or not is
their business, and if it's utterly stupid it'll be their fault if
it doesn't work as expected..

2008-08-10 13:49:18

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 2/4] integrity: special fs magic

Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:47:59 PM:


> On Fri, Aug 08, 2008 at 12:15:19PM -0700, Greg KH wrote:
> > On Fri, Aug 08, 2008 at 12:04:48PM -0700, Greg KH wrote:
> > > On Fri, Aug 08, 2008 at 02:55:42PM -0400, Mimi Zohar wrote:
> > > > - Move special fs magic number definitions to magic.h
> > > > - Add magic.h include
> > > >
> > > > Signed-off-by: Mimi Zohar <[email protected]>
> > >
> > > Why? What is this patch for? Are you going to do something with
these
> > > magic values later?
> >
> > Ok, I now see the follow-up patch that does something with them.
> >
> > You should say so in this patch.
> >
> > And is it really ok to be doing things from userspace based on a
> > filesystem "magic" key? Those are numbers we have never exported to
> > userspace before, what happens if they are changed?
>
> These constants re exported to userspace in struct statfs .type and
> better don't change. Providing the symbolic names for them sounds
> like a good idea to me. What userspace does with that or not is
> their business, and if it's utterly stupid it'll be their fault if
> it doesn't work as expected..

ok.

Mimi