Prev: cpqphp: NULL ptr deref in cpqhpc_probe
Next: Add scripts/coccinelle/kzalloc-simple.cocci
From: Roberto Sassu on 3 Jun 2010 06:00
Sorry for resending, the previous was rejected by some mailing list.


On Wednesday 02 June 2010 20:44:25 Mimi Zohar wrote:
> On Tue, 2010-06-01 at 19:05 +0200, Roberto Sassu wrote:
> > Description of the issue:
> >
> > The function 'simple_fill_super()' in the path 'fs/libfs.c' takes the 'magic' argument as int.
> > In the include file 'include/linux/fs.h' the 's_magic' field of the 'super_block' structure is
> > declared as unsigned long.
> > This causes a misbehaviour in the 'Integrity Measurement Architecture' security module,
> > since the 's_magic' field is used as criteria to determine if the inode must be measured.
>
> There aren't any magic numbers today greater than 32 bits. Out of
> curiosity, which magic number on which platform are you having a
> problem?
>

I'm using a Fedora 12 64-bit KVM virtual machine. I do some tests on the 'ima_must_measure()'
function and i noted that the result for inodes with superblock magic SELINUX_MAGIC is to
measure, when the action specified in the default policy is don't measure. So i modified
the code to display the superblock's magic of measured inodes adding this line in the function
'process_measurement()' in 'security/integrity/ima/ima_main.c' after 'ima_must_measure()':

printk("file %s: magic: %lx\n", file->f_dentry->d_name.name, inode->i_sb->s_magic);


I obtained this result:

...
file access: magic 0xfffffffff97cff8c
...


The magic that i'm expecting is 0xf97cff8c. I think this is why the IMA policy is not applied
correctly.
I investigated further the selinux's code to understand how the super_block structure is
instantiated in memory and i found this code in 'security/selinux/selinuxfs.c' , line 1601:

ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);

In the prototype of the above function the type of the second argument is 'int', when the
's_magic' type of the 'super_block' structure is 'unsigned long'.
In the patch i modified the type of the second argument of the function 'simple_fill_super()'.
This solves my problem but, since this is used by other filesystems, i don't known if this
solution is valid in general.

> > This patch applies to the mainline kernel repository.
> >
> >
> > >From a9f6d9bc7b2259ac025977f4b28a8b90784caf62 Mon Sep 17 00:00:00 2001
> > From: Roberto Sassu <roberto.sassu(a)polito.it>
> > Date: Tue, 1 Jun 2010 18:28:13 +0200
> > Subject: [PATCH] BUG: wrong type for magic argument in simple_fill_super(), fs/libfs.c
> >
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu(a)polito.it>
>
> Reviewed-by: Mimi Zohar <zohar(a)us.ibm.com>
>
> > ---
> > fs/libfs.c | 2 +-
> > include/linux/fs.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/libfs.c b/fs/libfs.c
> > index 09e1016..7d966e8 100644
> > --- a/fs/libfs.c
> > +++ b/fs/libfs.c
> > @@ -489,7 +489,7 @@ int simple_write_end(struct file *file, struct address_space *mapping,
> > * unique inode values later for this filesystem, then you must take care
> > * to pass it an appropriate max_reserved value to avoid collisions.
> > */
> > -int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
> > +int simple_fill_super(struct super_block *s, unsigned long magic, struct tree_descr *files)
> > {
> > struct inode *inode;
> > struct dentry *root;
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 3428393..471e1ff 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2388,7 +2388,7 @@ extern const struct file_operations simple_dir_operations;
> > extern const struct inode_operations simple_dir_inode_operations;
> > struct tree_descr { char *name; const struct file_operations *ops; int mode; };
> > struct dentry *d_alloc_name(struct dentry *, const char *);
> > -extern int simple_fill_super(struct super_block *, int, struct tree_descr *);
> > +extern int simple_fill_super(struct super_block *, unsigned long, struct tree_descr *);
> > extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count);
> > extern void simple_release_fs(struct vfsmount **mount, int *count);
> >
>
>
>
 | 
Pages: 1
Prev: cpqphp: NULL ptr deref in cpqhpc_probe
Next: Add scripts/coccinelle/kzalloc-simple.cocci