[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1174942604.12510.0.camel@localhost.localdomain>
Date: Mon, 26 Mar 2007 16:56:44 -0400
From: Mimi Zohar <zohar@...ux.vnet.ibm.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, safford@...son.ibm.com,
serue@...ux.vnet.ibm.com, kjhall@...ux.vnet.ibm.com,
zohar@...ibm.com
Subject: Re: [Patch 3/7] integrity: EVM as an integrity service provider
On Mon, 2007-03-26 at 13:23 -0500, Serge E. Hallyn wrote:
> Quoting Andrew Morton (akpm@...ux-foundation.org):
> > On Fri, 23 Mar 2007 12:09:36 -0400 Mimi Zohar <zohar@...ux.vnet.ibm.com> wrote:
> >
> > > This is a re-release of EVM as an integrity service provider.
> >
> > What a huge set of patches.
> >
> > Frankly, I don't know how we're going to get these reviewed and mergeable
> > and merged - there doesn't seem to be a lot of interest and personally
> > I only have a vague idea of what it all even does.
>
> Mimi,
>
> would it make sense to work with just the integrity subsystem first,
> then when they've been sitting for awhile without breaking anything
> and people are done giving comments, look at EVM, then after awhile at
> IMA?
>
> You've sent out all the patches so you can point people back to this
> submission if they ask "why isn't there a user" :)
>
> Just a thought. Cause it *is* a lot of patches...
>
> -serge
I don't have a problem with Serge's suggestion of including just the
integrity patches at this point, if that is preferable. The integrity
patches would be: integrity-new-hooks.patch,
integrity-new-hooks-fix.patch, integrity-fs-hook-placement.patch
and, the one I just submitted, integrity_dummy_verify_metadata.patch.
Then as Serge recommended, release the remainder of the code in smaller
pieces.
Mimi Zohar
> > This patch does worrisome-looking things with VFS internals (anything
> > which takes inode_lock is fishy).
> >
> >
> > Bunch of cleanups, pretty obvious:
> >
> > fs/sysfs/mount.c | 3 ---
> > include/linux/magic.h | 1 +
> > security/evm/evm.h | 2 --
> > security/evm/evm_config.c | 19 ++++++++++---------
> > security/evm/evm_crypto.c | 8 +++-----
> > security/evm/evm_main.c | 10 ++++------
> > 6 files changed, 18 insertions(+), 25 deletions(-)
> >
> > diff -puN fs/sysfs/mount.c~integrity-evm-as-an-integrity-service-provider-tidy fs/sysfs/mount.c
> > --- a/fs/sysfs/mount.c~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/fs/sysfs/mount.c
> > @@ -12,9 +12,6 @@
> >
> > #include "sysfs.h"
> >
> > -/* Random magic number */
> > -#define SYSFS_MAGIC 0x62656572
> > -
> > struct vfsmount *sysfs_mount;
> > struct super_block * sysfs_sb = NULL;
> > struct kmem_cache *sysfs_dir_cachep;
> > diff -puN include/linux/magic.h~integrity-evm-as-an-integrity-service-provider-tidy include/linux/magic.h
> > --- a/include/linux/magic.h~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/include/linux/magic.h
> > @@ -20,6 +20,7 @@
> > #define MINIX2_SUPER_MAGIC 0x2468 /* minix V2 fs */
> > #define MINIX2_SUPER_MAGIC2 0x2478 /* minix V2 fs, 30 char names */
> > #define MINIX3_SUPER_MAGIC 0x4d5a /* minix V3 fs */
> > +#define SYSFS_MAGIC 0x62656572
> >
> > #define MSDOS_SUPER_MAGIC 0x4d44 /* MD */
> > #define NCP_SUPER_MAGIC 0x564c /* Guess, what 0x564c is :-) */
> > diff -puN security/evm/evm.h~integrity-evm-as-an-integrity-service-provider-tidy security/evm/evm.h
> > --- a/security/evm/evm.h~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/security/evm/evm.h
> > @@ -8,7 +8,6 @@
> > #include <linux/spinlock_types.h>
> > #include <linux/integrity.h>
> >
> > -#define DEVFS_SUPER_MAGIC 0x1373
> > #define MAX_DIGEST_SIZE 20 /* 160-bits */
> >
> > extern char *evm_hmac, *evm_hash;
> > @@ -48,7 +47,6 @@ struct evm_iint_cache {
> > struct mutex mutex;
> > };
> >
> > -extern void display_config(const char *);
> > extern struct evm_xattr_config *evm_parse_config(char *data,
> > unsigned long datalen,
> > int *datasize);
> > diff -puN security/evm/evm_config.c~integrity-evm-as-an-integrity-service-provider-tidy security/evm/evm_config.c
> > --- a/security/evm/evm_config.c~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/security/evm/evm_config.c
> > @@ -18,17 +18,17 @@
> > * Configuration data
> > */
> > struct evm_xattr_config *evm_config_xattrdata;
> > -int evm_config_xattrnum = 0; /* number of extended attributes */
> > +int evm_config_xattrnum; /* number of extended attributes */
> >
> > /*
> > * inode->i_integrity information
> > */
> > -void display_config(const char *name)
> > +static void display_config(const char *name)
> > {
> > struct evm_xattr_config *config_p;
> >
> > for_each_xattr(config_p, evm_config_xattrdata, evm_config_xattrnum)
> > - printk(KERN_INFO "%s: %s\n", name, config_p->xattr_name);
> > + printk(KERN_INFO "%s: %s\n", name, config_p->xattr_name);
> > }
> >
> > /*
> > @@ -42,7 +42,6 @@ int evm_init_config(struct evm_xattr_con
> > evm_config_xattrdata = evm_data;
> > evm_config_xattrnum = evm_datasize;
> > display_config(__FUNCTION__);
> > -
> > } else {
> > printk(KERN_INFO "%s: config file definition missing\n",
> > __FUNCTION__);
> > @@ -60,9 +59,11 @@ static char *get_tag(char *buf_start, ch
> > /* Get start of tag */
> > while (bufp < buf_end) {
> > if (*bufp == ' ') /* skip blanks */
> > - while ((*bufp == ' ') && (bufp++ < buf_end)) ;
> > + while ((*bufp == ' ') && (bufp++ < buf_end))
> > + ;
> > else if (*bufp == '#') { /* skip comment */
> > - while ((*bufp != '\n') && (bufp++ < buf_end)) ;
> > + while ((*bufp != '\n') && (bufp++ < buf_end))
> > + ;
> > bufp++;
> > } else if (*bufp == '\n') /* skip newline */
> > bufp++;
> > @@ -107,8 +108,8 @@ struct evm_xattr_config *evm_parse_confi
> > *xattrnum = num_xattr;
> >
> > datap = data;
> > - config_xattrdata =
> > - kmalloc(num_xattr * sizeof(struct evm_xattr_config), GFP_KERNEL);
> > + config_xattrdata = kmalloc(num_xattr * sizeof(struct evm_xattr_config),
> > + GFP_KERNEL);
> > if (!config_xattrdata)
> > return NULL;
> >
> > @@ -123,7 +124,7 @@ struct evm_xattr_config *evm_parse_confi
> > return config_xattrdata;
> > }
> >
> > -inline void evm_cleanup_config(void)
> > +void evm_cleanup_config(void)
> > {
> > kfree(evm_config_xattrdata);
> > }
> > diff -puN security/evm/evm_crypto.c~integrity-evm-as-an-integrity-service-provider-tidy security/evm/evm_crypto.c
> > --- a/security/evm/evm_crypto.c~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/security/evm/evm_crypto.c
> > @@ -33,7 +33,7 @@
> > static unsigned char tpm_key[MAX_TPMKEY];
> > static int tpm_keylen = MAX_TPMKEY;
> >
> > -int update_file_hash(struct dentry *dentry, struct file *f,
> > +static int update_file_hash(struct dentry *dentry, struct file *f,
> > struct hash_desc *desc)
> > {
> > struct file *file = f;
> > @@ -217,11 +217,9 @@ int evm_calc_hmac(struct dentry *dentry,
> > struct scatterlist sg[1];
> > char *fname;
> > int error = 0;
> > -
> > struct evm_xattr_config *config_p;
> > int xattr_size = 0;
> > char *xattr_value = NULL;
> > -
> > struct h_misc {
> > unsigned long ino;
> > __u32 generation;
> > @@ -278,7 +276,7 @@ int evm_calc_hmac(struct dentry *dentry,
> > __FUNCTION__, fname,
> > config_p->xattr_name);
> > }
> > - };
> > + }
> > kfree(xattr_value);
> > memset(hmac_misc, 0, sizeof misc);
> > hmac_misc->ino = inode->i_ino;
> > @@ -331,7 +329,7 @@ int evm_init_tpmkernkey(void)
> >
> > kmk = request_key(&key_type_user, TPMKEY, NULL);
> > if (IS_ERR(kmk)) {
> > - return (-1);
> > + return -1;
> > } else {
> > down_read(&kmk->sem);
> > ukp = kmk->payload.data;
> > diff -puN security/evm/evm_main.c~integrity-evm-as-an-integrity-service-provider-tidy security/evm/evm_main.c
> > --- a/security/evm/evm_main.c~integrity-evm-as-an-integrity-service-provider-tidy
> > +++ a/security/evm/evm_main.c
> > @@ -24,6 +24,7 @@
> > #include <linux/proc_fs.h>
> > #include <linux/xattr.h>
> > #include <linux/file.h>
> > +#include <linux/magic.h>
> > #include <linux/writeback.h>
> > #include "evm_integrity.h"
> > #include "evm.h"
> > @@ -363,10 +364,7 @@ static int evm_verify_data(struct dentry
> > */
> > static int skip_measurement(struct inode *inode, int mask)
> > {
> > -#define SYSFS_MAGIC 0x62656572
> > -
> > - if ((inode->i_sb->s_magic == DEVFS_SUPER_MAGIC) ||
> > - (inode->i_sb->s_magic == PROC_SUPER_MAGIC) ||
> > + if ((inode->i_sb->s_magic == PROC_SUPER_MAGIC) ||
> > (inode->i_sb->s_magic == SYSFS_MAGIC)) {
> > return 1; /*can't measure */
> > }
> > @@ -877,9 +875,9 @@ static void evm_enable_integrity(void)
> >
> > static void evm_cleanup_integrity(void)
> > {
> > - if (evm_install) {
> > + if (evm_install)
> > unregister_integrity(&evm_install_ops);
> > - } else
> > + else
> > unregister_integrity(&evm_integrity_ops);
> > }
> >
> > _
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists