diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2010-10-26 11:37:48 -0700 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2010-10-26 11:37:48 -0700 |
commit | f9ba5375a8aae4aeea6be15df77e24707a429812 (patch) | |
tree | c6388d7e40f0f6a70d7ba6a4d4aeaa0d1f5591f6 | |
parent | 45352bbf48e95078b4acd20774f49e72676e1e0f (diff) | |
parent | bade72d607c4eb1b1d6c7852c493b75f065a56b5 (diff) | |
download | linux-3.10-f9ba5375a8aae4aeea6be15df77e24707a429812.tar.gz linux-3.10-f9ba5375a8aae4aeea6be15df77e24707a429812.tar.bz2 linux-3.10-f9ba5375a8aae4aeea6be15df77e24707a429812.zip |
Merge branch 'ima-memory-use-fixes'
* ima-memory-use-fixes:
IMA: fix the ToMToU logic
IMA: explicit IMA i_flag to remove global lock on inode_delete
IMA: drop refcnt from ima_iint_cache since it isn't needed
IMA: only allocate iint when needed
IMA: move read counter into struct inode
IMA: use i_writecount rather than a private counter
IMA: use inode->i_lock to protect read and write counters
IMA: convert internal flags from long to char
IMA: use unsigned int instead of long for counters
IMA: drop the inode opencount since it isn't needed for operation
IMA: use rbtree instead of radix tree for inode information cache
-rw-r--r-- | fs/inode.c | 1 | ||||
-rw-r--r-- | include/linux/fs.h | 6 | ||||
-rw-r--r-- | security/integrity/ima/ima.h | 18 | ||||
-rw-r--r-- | security/integrity/ima/ima_api.c | 2 | ||||
-rw-r--r-- | security/integrity/ima/ima_iint.c | 158 | ||||
-rw-r--r-- | security/integrity/ima/ima_main.c | 184 | ||||
-rw-r--r-- | security/security.c | 10 |
7 files changed, 202 insertions, 177 deletions
diff --git a/fs/inode.c b/fs/inode.c index 86464332e59..56d909d69bc 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -24,6 +24,7 @@ #include <linux/mount.h> #include <linux/async.h> #include <linux/posix_acl.h> +#include <linux/ima.h> /* * This is needed for the following functions: diff --git a/include/linux/fs.h b/include/linux/fs.h index 4f34ff6e555..bb20373d0b4 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -231,6 +231,7 @@ struct inodes_stat_t { #define S_NOCMTIME 128 /* Do not update file c/mtime */ #define S_SWAPFILE 256 /* Do not truncate: swapon got its bmaps */ #define S_PRIVATE 512 /* Inode is fs-internal */ +#define S_IMA 1024 /* Inode has an associated IMA struct */ /* * Note that nosuid etc flags are inode-specific: setting some file-system @@ -265,6 +266,7 @@ struct inodes_stat_t { #define IS_NOCMTIME(inode) ((inode)->i_flags & S_NOCMTIME) #define IS_SWAPFILE(inode) ((inode)->i_flags & S_SWAPFILE) #define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE) +#define IS_IMA(inode) ((inode)->i_flags & S_IMA) /* the read-only stuff doesn't really belong here, but any other place is probably as bad and I don't want to create yet another include file. */ @@ -772,6 +774,10 @@ struct inode { unsigned int i_flags; +#ifdef CONFIG_IMA + /* protected by i_lock */ + unsigned int i_readcount; /* struct files open RO */ +#endif atomic_t i_writecount; #ifdef CONFIG_SECURITY void *i_security; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 3fbcd1dda0e..ac79032bdf2 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -70,6 +70,7 @@ int ima_init(void); void ima_cleanup(void); int ima_fs_init(void); void ima_fs_cleanup(void); +int ima_inode_alloc(struct inode *inode); int ima_add_template_entry(struct ima_template_entry *entry, int violation, const char *op, struct inode *inode); int ima_calc_hash(struct file *file, char *digest); @@ -96,19 +97,16 @@ static inline unsigned long ima_hash_key(u8 *digest) } /* iint cache flags */ -#define IMA_MEASURED 1 +#define IMA_MEASURED 0x01 /* integrity data associated with an inode */ struct ima_iint_cache { + struct rb_node rb_node; /* rooted in ima_iint_tree */ + struct inode *inode; /* back pointer to inode in question */ u64 version; /* track inode changes */ - unsigned long flags; + unsigned char flags; u8 digest[IMA_DIGEST_SIZE]; struct mutex mutex; /* protects: version, flags, digest */ - long readcount; /* measured files readcount */ - long writecount; /* measured files writecount */ - long opencount; /* opens reference count */ - struct kref refcount; /* ima_iint_cache reference count */ - struct rcu_head rcu; }; /* LIM API function definitions */ @@ -122,13 +120,11 @@ int ima_store_template(struct ima_template_entry *entry, int violation, void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show); -/* radix tree calls to lookup, insert, delete +/* rbtree tree calls to lookup, insert, delete * integrity data associated with an inode. */ struct ima_iint_cache *ima_iint_insert(struct inode *inode); -struct ima_iint_cache *ima_iint_find_get(struct inode *inode); -void iint_free(struct kref *kref); -void iint_rcu_free(struct rcu_head *rcu); +struct ima_iint_cache *ima_iint_find(struct inode *inode); /* IMA policy related functions */ enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK }; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 52015d098fd..d3963de6003 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -116,7 +116,7 @@ int ima_must_measure(struct ima_iint_cache *iint, struct inode *inode, { int must_measure; - if (iint->flags & IMA_MEASURED) + if (iint && iint->flags & IMA_MEASURED) return 1; must_measure = ima_match_policy(inode, function, mask); diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c index afba4aef812..c442e47b678 100644 --- a/security/integrity/ima/ima_iint.c +++ b/security/integrity/ima/ima_iint.c @@ -12,98 +12,119 @@ * File: ima_iint.c * - implements the IMA hooks: ima_inode_alloc, ima_inode_free * - cache integrity information associated with an inode - * using a radix tree. + * using a rbtree tree. */ #include <linux/slab.h> #include <linux/module.h> #include <linux/spinlock.h> -#include <linux/radix-tree.h> +#include <linux/rbtree.h> #include "ima.h" -RADIX_TREE(ima_iint_store, GFP_ATOMIC); -DEFINE_SPINLOCK(ima_iint_lock); +static struct rb_root ima_iint_tree = RB_ROOT; +static DEFINE_SPINLOCK(ima_iint_lock); static struct kmem_cache *iint_cache __read_mostly; int iint_initialized = 0; -/* ima_iint_find_get - return the iint associated with an inode - * - * ima_iint_find_get gets a reference to the iint. Caller must - * remember to put the iint reference. +/* + * __ima_iint_find - return the iint associated with an inode */ -struct ima_iint_cache *ima_iint_find_get(struct inode *inode) +static struct ima_iint_cache *__ima_iint_find(struct inode *inode) { struct ima_iint_cache *iint; + struct rb_node *n = ima_iint_tree.rb_node; + + assert_spin_locked(&ima_iint_lock); + + while (n) { + iint = rb_entry(n, struct ima_iint_cache, rb_node); + + if (inode < iint->inode) + n = n->rb_left; + else if (inode > iint->inode) + n = n->rb_right; + else + break; + } + if (!n) + return NULL; - rcu_read_lock(); - iint = radix_tree_lookup(&ima_iint_store, (unsigned long)inode); - if (!iint) - goto out; - kref_get(&iint->refcount); -out: - rcu_read_unlock(); return iint; } -/** - * ima_inode_alloc - allocate an iint associated with an inode - * @inode: pointer to the inode +/* + * ima_iint_find - return the iint associated with an inode */ -int ima_inode_alloc(struct inode *inode) +struct ima_iint_cache *ima_iint_find(struct inode *inode) { - struct ima_iint_cache *iint = NULL; - int rc = 0; - - iint = kmem_cache_alloc(iint_cache, GFP_NOFS); - if (!iint) - return -ENOMEM; + struct ima_iint_cache *iint; - rc = radix_tree_preload(GFP_NOFS); - if (rc < 0) - goto out; + if (!IS_IMA(inode)) + return NULL; spin_lock(&ima_iint_lock); - rc = radix_tree_insert(&ima_iint_store, (unsigned long)inode, iint); + iint = __ima_iint_find(inode); spin_unlock(&ima_iint_lock); - radix_tree_preload_end(); -out: - if (rc < 0) - kmem_cache_free(iint_cache, iint); - return rc; + return iint; } -/* iint_free - called when the iint refcount goes to zero */ -void iint_free(struct kref *kref) +static void iint_free(struct ima_iint_cache *iint) { - struct ima_iint_cache *iint = container_of(kref, struct ima_iint_cache, - refcount); iint->version = 0; iint->flags = 0UL; - if (iint->readcount != 0) { - printk(KERN_INFO "%s: readcount: %ld\n", __func__, - iint->readcount); - iint->readcount = 0; - } - if (iint->writecount != 0) { - printk(KERN_INFO "%s: writecount: %ld\n", __func__, - iint->writecount); - iint->writecount = 0; - } - if (iint->opencount != 0) { - printk(KERN_INFO "%s: opencount: %ld\n", __func__, - iint->opencount); - iint->opencount = 0; - } - kref_init(&iint->refcount); kmem_cache_free(iint_cache, iint); } -void iint_rcu_free(struct rcu_head *rcu_head) +/** + * ima_inode_alloc - allocate an iint associated with an inode + * @inode: pointer to the inode + */ +int ima_inode_alloc(struct inode *inode) { - struct ima_iint_cache *iint = container_of(rcu_head, - struct ima_iint_cache, rcu); - kref_put(&iint->refcount, iint_free); + struct rb_node **p; + struct rb_node *new_node, *parent = NULL; + struct ima_iint_cache *new_iint, *test_iint; + int rc; + + new_iint = kmem_cache_alloc(iint_cache, GFP_NOFS); + if (!new_iint) + return -ENOMEM; + + new_iint->inode = inode; + new_node = &new_iint->rb_node; + + mutex_lock(&inode->i_mutex); /* i_flags */ + spin_lock(&ima_iint_lock); + + p = &ima_iint_tree.rb_node; + while (*p) { + parent = *p; + test_iint = rb_entry(parent, struct ima_iint_cache, rb_node); + + rc = -EEXIST; + if (inode < test_iint->inode) + p = &(*p)->rb_left; + else if (inode > test_iint->inode) + p = &(*p)->rb_right; + else + goto out_err; + } + + inode->i_flags |= S_IMA; + rb_link_node(new_node, parent, p); + rb_insert_color(new_node, &ima_iint_tree); + + spin_unlock(&ima_iint_lock); + mutex_unlock(&inode->i_mutex); /* i_flags */ + + return 0; +out_err: + spin_unlock(&ima_iint_lock); + mutex_unlock(&inode->i_mutex); /* i_flags */ + iint_free(new_iint); + + return rc; } /** @@ -116,11 +137,20 @@ void ima_inode_free(struct inode *inode) { struct ima_iint_cache *iint; + if (inode->i_readcount) + printk(KERN_INFO "%s: readcount: %u\n", __func__, inode->i_readcount); + + inode->i_readcount = 0; + + if (!IS_IMA(inode)) + return; + spin_lock(&ima_iint_lock); - iint = radix_tree_delete(&ima_iint_store, (unsigned long)inode); + iint = __ima_iint_find(inode); + rb_erase(&iint->rb_node, &ima_iint_tree); spin_unlock(&ima_iint_lock); - if (iint) - call_rcu(&iint->rcu, iint_rcu_free); + + iint_free(iint); } static void init_once(void *foo) @@ -131,10 +161,6 @@ static void init_once(void *foo) iint->version = 0; iint->flags = 0UL; mutex_init(&iint->mutex); - iint->readcount = 0; - iint->writecount = 0; - iint->opencount = 0; - kref_init(&iint->refcount); } static int __init ima_iintcache_init(void) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index e662b89d407..203de979d30 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -85,50 +85,6 @@ out: return found; } -/* ima_read_write_check - reflect possible reading/writing errors in the PCR. - * - * When opening a file for read, if the file is already open for write, - * the file could change, resulting in a file measurement error. - * - * Opening a file for write, if the file is already open for read, results - * in a time of measure, time of use (ToMToU) error. - * - * In either case invalidate the PCR. - */ -enum iint_pcr_error { TOMTOU, OPEN_WRITERS }; -static void ima_read_write_check(enum iint_pcr_error error, - struct ima_iint_cache *iint, - struct inode *inode, - const unsigned char *filename) -{ - switch (error) { - case TOMTOU: - if (iint->readcount > 0) - ima_add_violation(inode, filename, "invalid_pcr", - "ToMToU"); - break; - case OPEN_WRITERS: - if (iint->writecount > 0) - ima_add_violation(inode, filename, "invalid_pcr", - "open_writers"); - break; - } -} - -/* - * Update the counts given an fmode_t - */ -static void ima_inc_counts(struct ima_iint_cache *iint, fmode_t mode) -{ - BUG_ON(!mutex_is_locked(&iint->mutex)); - - iint->opencount++; - if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) - iint->readcount++; - if (mode & FMODE_WRITE) - iint->writecount++; -} - /* * ima_counts_get - increment file counts * @@ -145,62 +101,101 @@ void ima_counts_get(struct file *file) struct dentry *dentry = file->f_path.dentry; struct inode *inode = dentry->d_inode; fmode_t mode = file->f_mode; - struct ima_iint_cache *iint; int rc; + bool send_tomtou = false, send_writers = false; - if (!iint_initialized || !S_ISREG(inode->i_mode)) + if (!S_ISREG(inode->i_mode)) return; - iint = ima_iint_find_get(inode); - if (!iint) - return; - mutex_lock(&iint->mutex); + + spin_lock(&inode->i_lock); + if (!ima_initialized) goto out; - rc = ima_must_measure(iint, inode, MAY_READ, FILE_CHECK); - if (rc < 0) - goto out; if (mode & FMODE_WRITE) { - ima_read_write_check(TOMTOU, iint, inode, dentry->d_name.name); + if (inode->i_readcount && IS_IMA(inode)) + send_tomtou = true; goto out; } - ima_read_write_check(OPEN_WRITERS, iint, inode, dentry->d_name.name); + + rc = ima_must_measure(NULL, inode, MAY_READ, FILE_CHECK); + if (rc < 0) + goto out; + + if (atomic_read(&inode->i_writecount) > 0) + send_writers = true; out: - ima_inc_counts(iint, file->f_mode); - mutex_unlock(&iint->mutex); + /* remember the vfs deals with i_writecount */ + if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) + inode->i_readcount++; - kref_put(&iint->refcount, iint_free); + spin_unlock(&inode->i_lock); + + if (send_tomtou) + ima_add_violation(inode, dentry->d_name.name, "invalid_pcr", + "ToMToU"); + if (send_writers) + ima_add_violation(inode, dentry->d_name.name, "invalid_pcr", + "open_writers"); } /* * Decrement ima counts */ -static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode, - struct file *file) +static void ima_dec_counts(struct inode *inode, struct file *file) { mode_t mode = file->f_mode; - BUG_ON(!mutex_is_locked(&iint->mutex)); - iint->opencount--; - if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) - iint->readcount--; - if (mode & FMODE_WRITE) { - iint->writecount--; - if (iint->writecount == 0) { - if (iint->version != inode->i_version) - iint->flags &= ~IMA_MEASURED; + assert_spin_locked(&inode->i_lock); + + if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) { + if (unlikely(inode->i_readcount == 0)) { + if (!ima_limit_imbalance(file)) { + printk(KERN_INFO "%s: open/free imbalance (r:%u)\n", + __func__, inode->i_readcount); + dump_stack(); + } + return; } + inode->i_readcount--; } +} - if (((iint->opencount < 0) || - (iint->readcount < 0) || - (iint->writecount < 0)) && - !ima_limit_imbalance(file)) { - printk(KERN_INFO "%s: open/free imbalance (r:%ld w:%ld o:%ld)\n", - __func__, iint->readcount, iint->writecount, - iint->opencount); - dump_stack(); - } +static void ima_check_last_writer(struct ima_iint_cache *iint, + struct inode *inode, + struct file *file) +{ + mode_t mode = file->f_mode; + + BUG_ON(!mutex_is_locked(&iint->mutex)); + assert_spin_locked(&inode->i_lock); + + if (mode & FMODE_WRITE && + atomic_read(&inode->i_writecount) == 1 && + iint->version != inode->i_version) + iint->flags &= ~IMA_MEASURED; +} + +static void ima_file_free_iint(struct ima_iint_cache *iint, struct inode *inode, + struct file *file) +{ + mutex_lock(&iint->mutex); + spin_lock(&inode->i_lock); + + ima_dec_counts(inode, file); + ima_check_last_writer(iint, inode, file); + + spin_unlock(&inode->i_lock); + mutex_unlock(&iint->mutex); +} + +static void ima_file_free_noiint(struct inode *inode, struct file *file) +{ + spin_lock(&inode->i_lock); + + ima_dec_counts(inode, file); + + spin_unlock(&inode->i_lock); } /** @@ -208,7 +203,7 @@ static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode, * @file: pointer to file structure being freed * * Flag files that changed, based on i_version; - * and decrement the iint readcount/writecount. + * and decrement the i_readcount. */ void ima_file_free(struct file *file) { @@ -217,14 +212,14 @@ void ima_file_free(struct file *file) if (!iint_initialized || !S_ISREG(inode->i_mode)) return; - iint = ima_iint_find_get(inode); - if (!iint) - return; - mutex_lock(&iint->mutex); - ima_dec_counts(iint, inode, file); - mutex_unlock(&iint->mutex); - kref_put(&iint->refcount, iint_free); + iint = ima_iint_find(inode); + + if (iint) + ima_file_free_iint(iint, inode, file); + else + ima_file_free_noiint(inode, file); + } static int process_measurement(struct file *file, const unsigned char *filename, @@ -236,11 +231,21 @@ static int process_measurement(struct file *file, const unsigned char *filename, if (!ima_initialized || !S_ISREG(inode->i_mode)) return 0; - iint = ima_iint_find_get(inode); - if (!iint) - return -ENOMEM; + + rc = ima_must_measure(NULL, inode, mask, function); + if (rc != 0) + return rc; +retry: + iint = ima_iint_find(inode); + if (!iint) { + rc = ima_inode_alloc(inode); + if (!rc || rc == -EEXIST) + goto retry; + return rc; + } mutex_lock(&iint->mutex); + rc = ima_must_measure(iint, inode, mask, function); if (rc != 0) goto out; @@ -250,7 +255,6 @@ static int process_measurement(struct file *file, const unsigned char *filename, ima_store_measurement(iint, file, filename); out: mutex_unlock(&iint->mutex); - kref_put(&iint->refcount, iint_free); return rc; } diff --git a/security/security.c b/security/security.c index b50f472061a..3ef5e2a7a74 100644 --- a/security/security.c +++ b/security/security.c @@ -325,16 +325,8 @@ EXPORT_SYMBOL(security_sb_parse_opts_str); int security_inode_alloc(struct inode *inode) { - int ret; - inode->i_security = NULL; - ret = security_ops->inode_alloc_security(inode); - if (ret) - return ret; - ret = ima_inode_alloc(inode); - if (ret) - security_inode_free(inode); - return ret; + return security_ops->inode_alloc_security(inode); } void security_inode_free(struct inode *inode) |