aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2010-10-26 11:37:48 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2010-10-26 11:37:48 -0700
commitf9ba5375a8aae4aeea6be15df77e24707a429812 (patch)
treec6388d7e40f0f6a70d7ba6a4d4aeaa0d1f5591f6
parent45352bbf48e95078b4acd20774f49e72676e1e0f (diff)
parentbade72d607c4eb1b1d6c7852c493b75f065a56b5 (diff)
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.c1
-rw-r--r--include/linux/fs.h6
-rw-r--r--security/integrity/ima/ima.h18
-rw-r--r--security/integrity/ima/ima_api.c2
-rw-r--r--security/integrity/ima/ima_iint.c158
-rw-r--r--security/integrity/ima/ima_main.c184
-rw-r--r--security/security.c10
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)